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 defaults.py #539

Merged
merged 3 commits into from
Mar 7, 2024
Merged

Update defaults.py #539

merged 3 commits into from
Mar 7, 2024

Conversation

ptth222
Copy link

@ptth222 ptth222 commented Feb 22, 2024

The previous regex didn't match any DOI I tried. I found this one in a blog post from Crossref where they say it matched 74.4M out of 74.9M DOIs that they have seen. The post is here: https://www.crossref.org/blog/dois-and-matching-regular-expressions/

@@ -17,14 +17,14 @@ def test_b_ii_s_3(self):
data_path = path.join(path.dirname(path.abspath(__file__)), '..', '..', 'data', 'tab', 'BII-S-3')
with open(path.join(data_path, 'i_gilbert.txt'), 'r') as data_file:
r = validate(fp=data_file, config_dir=self.default_conf, origin="")
self.assertEqual(len(r['warnings']), 12)
self.assertEqual(len(r['warnings']), 10)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two DOIs were actually valid, this is why there's only 10 warnings left instead of 12

The previous regex didn't match any DOI I tried. I found this one in a blog post from Crossref where they say it matched 74.4M out of 74.9M DOIs that they have seen. The post is here: https://www.crossref.org/blog/dois-and-matching-regular-expressions/
Since the DOI regex works now there are 2 fewer warnings. Previously it was warning about valid DOIs.
@ptth222 ptth222 changed the base branch from develop to issue-511 March 6, 2024 19:26
@ptth222
Copy link
Author

ptth222 commented Mar 6, 2024

I rebased to issue-511. It has the same changes that are on #549, so if that is merged first there should only be the changes to the DOI regex and 1 test.

@coveralls
Copy link

Coverage Status

coverage: 81.299%. remained the same
when pulling 2275ee9 on ptth222:doi-regex-fix
into 8aae210 on ISA-tools:issue-511.

@terazus terazus merged commit e5f6fd2 into ISA-tools:issue-511 Mar 7, 2024
5 checks passed
@ptth222 ptth222 deleted the doi-regex-fix branch March 20, 2024 22:27
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.

4 participants