-
Notifications
You must be signed in to change notification settings - Fork 3
Modernize #2
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
base: master
Are you sure you want to change the base?
Modernize #2
Conversation
Also, this repository doesn't seem to have issues enabled. It would be good to turn that on as well? |
Any progress on getting this merged? I don't see anything that should cause issue. |
I'm afraid I don't have access to this repo to merge it. @alejandrogallo Any chance you can find some time to give it a look and merge it? 😁 EDIT: As far as I can tell the tests still pass (locally), but it's probably a good idea to re-run the CI anyway before merging. EDIT2: Enabled support for testing on Python 3.13 as well! 😁 |
Trying to use this, I notice I needed to pass
Although the Anything left to get this merged @alejandrogallo ? |
... Actually, it turns out that on my system, this PR suffices to get the tests to pass, with no other trickery needed. I don't know what happened in the last two months to make my bug no longer reproduce, but it seems #3 can be closed without merging it (or merged if we want that redundancy, though we'd probably want to drop the commit introducing |
Ah, I see what happened -- 844fdfd fixed the urls the tests point to, making the tests no longer break |
Just looked at
I think it's ok to change these because they're just checking that the DOI parsing works as expected. However, we might want to add some more tests to check that we manage to follow redirects properly and whatnot. Maybe a follow-up to #3 could go in that direction? |
Yeah, I see it's just checking the https://doi.org/api/handles/ endpoint works as expected. I am trying to investigate getting the redirect code to work for robustness' sake, but OTOH given the 403's the redirected URLs are giving, I'm suspecting some of these sites aren't very scraper-friendly, and so are likely to be difficult to support. |
This updates
python-doi
to be a bit more in line withpapis
and generally work with newer linting and stuff.The commits should be fairly self-contained. Let me know if you spot anything wrong!
I have some followup things to clean up the docs as well.