-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new entities: funders, sources, publishers #130
Conversation
@maelle Have you seen this error for other ropensci packages? This is in our
|
I haven't seen this error but I think your workflow can just skip installing rotemplate, only pkgdown is needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just 2 small comments
Also apparently I can't comment on unchanged files in my review, but we should also edit the Lines 59 to 68 in 66f0743
And maybe worth adding the failing ones here to tests once that's implemented: # These work:
i <- oa_fetch(identifier = "I4200000001") # Institution
a <- oa_fetch(identifier = "A1969205032") # Author
# New ones don't yet:
f <- oa_fetch(identifier = "F4320332161") # Funder
#> Error in match.arg(entity, oa_entities()): 'arg' must be NULL or a character vector
p <- oa_fetch(identifier = "P4310311775") # Publisher
#> Error in match.arg(entity, oa_entities()): 'arg' must be NULL or a character vector
s <- oa_fetch(identifier = "S1983995261") # Source
#> Error in match.arg(entity, oa_entities()): 'arg' must be NULL or a character vector |
Ready for next round of review @yjunechoe 🙏🏽 |
All looks good to me! - You can go ahead and merge One thing to watch out for which I think might just be on the API end is that multiple entity search works for Source but not Funder and Publisher. Maybe they haven't implemented the OR filter by ID for them yet: ## These work
w2 <- oa_fetch(identifier = c("W2100837269", "W1775749144"))
s2 <- oa_fetch(identifier = c("S2764455111", "S4306400806"))
## These don't work
f2 <- oa_fetch(identifier = c("F4320332161", "F4320306076"))
#> Error: OpenAlex API request failed [403]
#> Invalid query parameters error.
#> <openalex_id is not a valid field. Valid fields are underscore or hyphenated versions of: cited_by_count, continent, country_code, default.search, description.search, display_name, display_name.search, from_created_date, grants_count, ids.crossref, ids.doi, ids.openalex, ids.ror, ids.wikidata, is_global_south, openalex, roles.id, ror, summary_stats.2yr_mean_citedness, summary_stats.h_index, summary_stats.i10_index, wikidata, works_count>
p2 <- oa_fetch(identifier = c("P4310311775", "P4310320990"))
#> Error: OpenAlex API request failed [403]
#> Invalid query parameters error.
#> <openalex_id is not a valid field. Valid fields are underscore or hyphenated versions of: cited_by_count, continent, country_codes, default.search, display_name, display_name.search, from_created_date, hierarchy_level, ids.openalex, ids.ror, ids.wikidata, lineage, openalex, parent_publisher, roles.id, ror, summary_stats.2yr_mean_citedness, summary_stats.h_index, summary_stats.i10_index, wikidata, works_count> For example, the code for But the code for Maybe OpenAlex will catch up on these soon :) |
Thank you for pointing this out, @yjunechoe! I notified the OpenAlex team. Hopefully, they'll fix this soon. 🤞🏽 |
OpenAlex team member here. I have added the "openalex_id" filter to funders and publishers, which should fix this issue. Just FYI, though, I believe that that filter has been deprecated in favor of "openalex" (e.g., https://api.openalex.org/funders?filter=openalex:https://openalex.org/F4320306076), which is probably why it wasn't included in those newer entities. |
* remove col_df, add open_access, closes #134 * refine aggregation * modify test * unnest open_access column * replace openalex_id with openalex #130 (comment)
Closes #124