Skip to content
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

Update empc_ftxt() to indicate 1 input limit #55

Merged
merged 2 commits into from
Nov 20, 2023

Conversation

allenbaron
Copy link
Contributor

epmc_ftxt() only works with one PMCID. Currently, it attempts to execute with more than one PMCID and fails. In the example below 2 good PMCIDs are passed, first together then separately. Only when they are given separately does it work.

library(europepmc)

pmcids <- c("PMC10341083", "PMC10297282")
res1 <- epmc_ftxt(pmcids)
#> Warning in if (!grepl("^PMC", ext_id)) stop("Please provide a PMCID, i.e. ids
#> starting with 'PMC'"): the condition has length > 1 and only the first element
#> will be used
#> Request failed [404]. Retrying in 1 seconds...
#> Request failed [404]. Retrying in 4 seconds...
#> Error in epmc_ftxt(pmcids): Not Found (HTTP 404). Failed to retrieve full text..

res2 <- lapply(pmcids, epmc_ftxt)
length(res2)
#> [1] 2

Created on 2023-11-17 by the reprex package (v2.0.1)

The current documentation appears to indicate epmc_ftxt() can accept multiple inputs and the test to ensure ext_id is correctly formatted will pass as long as the first element of ext_id starts with 'PMC' (all others are ignored with a warning).

This PR depluralizes the documentation and updates the ext_id argument test to ensure it allows only one PMCID.
Loosely related to #38 and #37.

It seems like the documentation for europepmc functions are written in plural while primarily accepting singular inputs, so I totally understand if the documentation updates are dropped. The ext_id test update alone would still avoid malformed API requests and the need for user debugging.

@njahn82
Copy link
Member

njahn82 commented Nov 20, 2023

Looks good, thanks for improving the documentation and input control; it was very confusing indeed.

@njahn82 njahn82 merged commit 86ccdfc into ropensci:main Nov 20, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants