-
Notifications
You must be signed in to change notification settings - Fork 155
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
[WIP] Process references #73
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. How were you thinking of splitting the functions? The existing implementation looks pretty efficient but it would be nice to be able to add a processor by just adding a function (like it was before I threw a spanner in the works!)
Otherwise, I only have an administrative comment.
setup.py
Outdated
@@ -17,7 +17,7 @@ | |||
author="Research Computing Service, Imperial College London", | |||
author_email="[email protected]", | |||
url="https://github.com/ImperialCollegeLondon/R2T2", | |||
install_requires=["wrapt"], | |||
install_requires=["wrapt", "bibtexparser"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we not moved to poetry? Should this not go in pyproject.toml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should, but #67 needs to be merged first.
This adds the processing of the references depending on their type (plain, doi and bibkey).
I'm working in changing the current implementation to separate functions and adding tests, but I wanted to leave this here as a WIP to get some feedback from you all, specially from @ChasNelson1990 who was working on using bibtex.