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

get_pmc_citeproc: switch to Literature Citation Exporter #80

Merged
merged 5 commits into from
Nov 15, 2018

Conversation

dhimmel
Copy link
Member

@dhimmel dhimmel commented Nov 14, 2018

NCBI Citaiton Exporter was taken offline without notice:
https://twitter.com/dhimmel/status/1061787168820092929

@dhimmel dhimmel requested a review from agitter November 14, 2018 19:52
@dhimmel
Copy link
Member Author

dhimmel commented Nov 14, 2018

@agitter this should take precedence over other PRs, since currently any pmcid citations will fail.

Currently this PR only switches get_pmc_citeproc and not get_pubmed_citeproc to use the Literature Citation Exporter. We can migrate get_pubmed_citeproc in the future, but it's not pressing.

Once merged, we should update rootstock and release Manubot 0.2.1 (I will start preparing release notes).

@dhimmel dhimmel mentioned this pull request Nov 14, 2018
Copy link
Member

@agitter agitter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we want to merge this quickly, so I would be okay merging as-is. However, the error message for an invalid identifier that begins with PMC and an empty or non-numeric suffix is not very informative.

$ manubot cite pmcid:PMC
## ERROR
'PMCID'
[]
## CRITICAL
Failure: exiting with code 1 due to logged errors

Other bad example cases include pmcid:PMCA and pmcid:PMC123A. The API provides a different response for invalid numeric identifiers, like pmcid:PMC3, which is caught and explained well by Manubot.

@dhimmel
Copy link
Member Author

dhimmel commented Nov 15, 2018

Ah interesting. When querying https://api.ncbi.nlm.nih.gov/lit/ctxp/v1/pmc/?format=csl&id= the following JSON is returned:

{
    "status": "error",
    "reason": {
        "id": [
            [
                "This field is required."
            ]
        ]
    }
}

However, when querying an invalid id such as https://api.ncbi.nlm.nih.gov/lit/ctxp/v1/pmc/?format=csl&id=3, the following XML is returned:

<pub-one-record-set xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:mml="http://www.w3.org/1998/Math/MathML" xmlns:p1="http://pubmed.gov/pub-one" p1:error="PubOne failure: empty XML."/>

@dhimmel
Copy link
Member Author

dhimmel commented Nov 15, 2018

@agitter I made the error checking and messaging more robust. Take a look again?

Copy link
Member

@agitter agitter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this version looks great. I tested with PMC0, PMC1, and PMCA to see the various error handling paths.

@dhimmel dhimmel merged commit 53bb682 into manubot:master Nov 15, 2018
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.

None yet

2 participants