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.

Sign up for GitHub

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

Jump to bottom

Validate citation identifiers conform to known identifier patterns #76

Merged
merged 14 commits into from
Nov 13, 2018
Merged

Validate citation identifiers conform to known identifier patterns #76

merged 14 commits into from
Nov 13, 2018

Conversation

dhimmel
Copy link
Member

@dhimmel dhimmel commented Oct 19, 2018 •
edited
Loading

Closes #71

dhimmel
dhimmel commented Oct 22, 2018
View reviewed changes
manubot/cite/util.py Outdated
'PubMed Identifiers should start with digits rather than PMC. '
f'Should {citation} switch the citation source to `pmcid`?'
)
if not regexes['pmid'].fullmatch(identifier):
Copy link
Member Author

Choose a reason for hiding this comment

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

change to elif

manubot/cite/util.py Outdated
f'Invalid DOI citation: {citation}\n'
'DOIs must start with `10.`.'
)
if not regexes['doi'].fullmatch(identifier):
Copy link
Member Author

Choose a reason for hiding this comment

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

change to elif

@dhimmel dhimmel force-pushed the citation-validator branch 3 times, most recently from 1bae7fc to e1e0d7c Compare October 24, 2018 15:19
@dhimmel dhimmel requested a review from agitter November 13, 2018 14:35
agitter
agitter approved these changes Nov 13, 2018
View reviewed changes
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.

This looks good to me. I have a few questions prompted by my local testing, but they do not necessarily require changes.

manubot/cite/cite_command.py Show resolved Hide resolved
manubot/cite/util.py Outdated Show resolved Hide resolved
@dhimmel
Detect source capitalization issues
76092d5
@dhimmel
is_valid_citation_string enable manuscript_extras option
edec38e 
Properly distinguishes the validation needs of manuscript citation
strings versus cite command citation strings.
@dhimmel
split manuscript_extras into 3 args
bac228b
@dhimmel
Copy link
Member Author

dhimmel commented Nov 13, 2018

@agitter great suggestions. I think you will be pleasantly surprised by the last three commits and their ability to address your comments!

agitter
agitter reviewed Nov 13, 2018
View reviewed changes
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.

These updates are great. As an error-prone user, I love informative error messages.

manubot/cite/util.py Outdated Show resolved Hide resolved
manubot/cite/util.py Show resolved Hide resolved
@dhimmel dhimmel merged commit f9673fe into manubot:master Nov 13, 2018
@dhimmel dhimmel deleted the citation-validator branch November 13, 2018 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agitter agitter agitter approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dhimmel @agitter