-
Notifications
You must be signed in to change notification settings - Fork 7
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 Papis Sci-Hub plugin #37
base: master
Are you sure you want to change the base?
Conversation
This pull request introduces 1 alert and fixes 2 when merging c50f34d into 4d07504 - view on LGTM.com new alerts:
fixed alerts:
|
Hello @raj-magesh thank you very much for this, I'll test it but I'm not using python 3.10. Papis tries to be compatible with py36 upwards, maybe it'll change in the future. Cheers! |
Sorry, I've been busy, but I'll get to this when I get some time! Shouldn't be too hard to port to Python 3.6. |
While trying to package this for Arch Linux, noticed the Attached is a patch on top of the above making it |
In the meantime we decided to drop 3.6, yes, so this should be accepted, I just have no time resources now to test this specific pull request, is someone available to referee this pr and approve the merge? Maybe @hseg you can add your diff to the pr? |
You mean open a fresh PR with the new diff?
Just tried doing that, turns out the PR needs to be rebased onto master.
Will see how far I can get on it tonight (turns out I have a little more time
than I thought).
…On Mon, Nov 13, 2023 at 12:26:35PM -0800, Alejandro Gallo wrote:
In the meantime we decided to drop 3.6, yes, so this should be accepted, I just have no time resources now to test this specific pull request, is someone available to referee this pr and approve the merge? Maybe @hseg you can add your diff to the pr?
--
Reply to this email directly or view it on GitHub:
#37 (comment)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
Correction: got confused by the extensive rewrite of |
However, just noticed PR also removes the liability warning, restoring it. |
@alejandrogallo ping on merging? (or, well, merging #62) |
I've created a Sci-Hub Downloader that's loaded as an
Importer
entry point. I've been using it without issues. Please let me know if this is fine.One thing to note is that this downloads BibTeX metadata directly from
https://doi.org
and some of the fields that I get from theDOI
importer are missing. The formatting is also slightly different (e.g. theyear
field is'2019'
instead of2019
. It's fine for my purposes but let me know if there's a better, more consistent way to do this.