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

WIP Add duecredit entries #1466

Merged
merged 32 commits into from
Sep 6, 2016
Merged

WIP Add duecredit entries #1466

merged 32 commits into from
Sep 6, 2016

Conversation

alexsavio
Copy link
Contributor

@alexsavio alexsavio commented May 3, 2016

This PR is related with #1464.
This is a WIP, I will be adding DOI entries to the code base.
Any help, comment or suggestion is appreciated.

With this first commit, I get:

$ python -m duecredit examples/test_spm.py

DueCredit Report:
- Access a cacophony of neuro-imaging file formats / nibabel (v 2.0.2) [1]
- Nipype: a flexible, lightweight and extensible neuroimaging data processing framework in Python / nipype (v 0.12.0-rc1) [2]
- Scientific tools library / numpy (v 1.11) [3]
- Scientific tools library / scipy (v 0.17) [4]

4 packages cited
0 modules cited
0 functions cited

References
----------

[1] Url('http://nipy.org/nibabel', key='http://nipy.org/nibabel')
[2] Gorgolewski, K. et al., 2011. Nipype: A Flexible, Lightweight and Extensible Neuroimaging Data Processing Framework in Python. Frontiers in Neuroinformatics, 5.
[3] Van Der Walt, S., Colbert, S.C. & Varoquaux, G., 2011. The NumPy array: a structure for efficient numerical computation. Computing in Science & Engineering, 13(2), pp.22–30.
[4] Jones, E. et al., 2001. SciPy: Open source scientific tools for Python.

@alexsavio alexsavio changed the title [WIP]: Add duecredict entries WIP Add duecredict entries May 3, 2016
@alexsavio alexsavio changed the title WIP Add duecredict entries WIP Add duecredit entries May 3, 2016
# Use duecredit (duecredit.org) to provide a citation to relevant work to
# be cited. This does nothing, unless the user has duecredit installed,
# And calls this with duecredit (as in `python -m duecredit script.py`):
due.cite(Doi("10.3389/fninf.2011.00013"),
Copy link
Member

Choose a reason for hiding this comment

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

this should now be replaced with the zenodo doi.

@alexsavio
Copy link
Contributor Author

Hi @satra, thanks!

The zenodo DOI changes for each version, doesn't it?

Wouldn't it be a good idea to put both references, the paper that is already there and also the Zenodo DOI?

@satra
Copy link
Member

satra commented May 3, 2016

yes - we can update the zenodo reference with each release. we can have both doi's in there.

@alexsavio
Copy link
Contributor Author

I hope you don't mind I go slowly with this. I had a problem with duecredit and the Zenodo DOI.

I have one question though...some nodes have references depending on trait values. Such as:
http://nipype.readthedocs.io/en/latest/interfaces/generated/nipype.interfaces.nipy.preprocess.html#spacetimerealigner

Do you think I should go deep as checking trait values to know what to cite?

@satra
Copy link
Member

satra commented May 4, 2016

@alexsavio - take your time with this or we can try to figure out a way to crowdsource this.

regarding your question about trait_values, due you think we could simply do this based on doi? then we can add a metadata field to the interface called dois, which we can update in such a case. we can add the doi extraction via duecredit during the help generation or runtime.

@alexsavio
Copy link
Contributor Author

Hi @satra, thanks for the follow-up. I have been reading the duecredit tests to understand it.
IMO we would need more information than the Doi to add a citation and keep the citations clear enough around the code.

I am thinking if we should keep all the citations together in one file in nipype or we should use decorators all around the code base. Given the size of nipype, IMO the "one file" approach would be easier to maintain and would keep the rest of the code base clean. If the extra fields in the dcite function work well, this could be possible.

More info below:

Following @arokem approach, apart from the DOI, he is using 3 more fields more for each citation: description, tags and path.

due.cite(Doi("10.1167/13.9.30"),
         description="Template project for small scientific Python projects",
         tags=["reference-implementation"],
         path='shablona')

description is used to describe the citation. If we have to manage 2 or 3 DOIs in the same place, this would be valuable.
path is a way to tell duecredit what module, class and/or function does the citation refers to. This is also used in the duecredit report for stats. I think there should be an automatic way of picking the path when using the @due.dcite decorator, but I could not find if this is possible.
tags: I don't understand the role of the tags yet.

And there is another field called conditions, which needs a dictionary which checks for function argument values to cite only if these conditions are met. Which can be quite useful in nipype's case.

There is some environment variables that can be implemented:
DUECREDIT_STYLE (default: 'harvard1') to tell citeproc which citation style to use,
DUECREDIT_REPORT_TAGS (default: 'reference-implementation,implementation')
and others: ['DUECREDIT_KEEPTEMP'].

I would add:
DUECREDIT_KEEPFILE (default: False) which would remove the .duecredit.p file if not set. Because I have to delete it every time I remove a due.cite entry from the code to test it, otherwise the citation still appears in the report.

@arokem
Copy link
Member

arokem commented May 4, 2016

Yes - I got that from @yarikoptic (thanks again!).

For the record, as I just discovered yesterday, you can also give it a full
bibtex entry instead of the DOI, which ends up being useful in some cases.
For example: scikit-learn-contrib/forest-confidence-interval#5

On Wed, May 4, 2016 at 8:27 AM, Alexandre M. S. notifications@github.com
wrote:

Hi @satra https://github.com/satra, thanks for the follow-up. I have
been reading the duecredit tests to understand it.
IMO we would need more information than the Doi to add a citation and keep
the citations clear enough around the code.

I am thinking if we should keep all the citations together in one file in
nipype or we should use decorators all around the code base. Given the size
of nipype, IMO the "one file" approach would be easier to maintain and
would keep the rest of the code base clean. If the extra fields in the
dcite function work well, this could be possible.

More info below:

Following @arokem https://github.com/arokem approach, apart from the
DOI, he is using 3 more fields more for each citation: description, tags
and path.

due.cite(Doi("10.1167/13.9.30"),
description="Template project for small scientific Python projects",
tags=["reference-implementation"],
path='shablona')

description is used to describe the citation. If we have to manage 2 or 3
DOIs in the same place, this would be valuable.
path is a way to tell duecredit what module, class and/or function does
the citation refers to. This is also used in the duecredit report for
stats. I think there should be an automatic way of picking the path when
using the @due.dcite decorator, but I could not find if this is possible.
tags: I don't understand the role of the tags yet.

And there is another field called conditions, which needs a dictionary
which checks for function argument values to cite only if these conditions
are met. Which can be quite useful in nipype's case.

There is some environment variables that can be implemented:
DUECREDIT_STYLE (default: 'harvard1') to tell citeproc which citation
style to use,
DUECREDIT_REPORT_TAGS (default: 'reference-implementation,implementation')
and others: ['DUECREDIT_KEEPTEMP'].

I would add:
DUECREDIT_KEEPFILE (default: False) which would remove the .duecredit.p
file if not set. Because I have to delete it every time I remove a due.cite
entry from the code to test it, otherwise the citation still appears in the
report.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1466 (comment)

@yarikoptic
Copy link
Member

I have sent a quick PR resolving some of the issues here... see alexsavio#8

Fix up import of version info from nipype/info.py
@coveralls
Copy link

coveralls commented May 4, 2016

Coverage Status

Coverage decreased (-0.002%) to 72.475% when pulling 8e2fb00 on alexsavio:enh/duecredit into 238fec1 on nipy:master.

@yarikoptic
Copy link
Member

and one more PR for completeness: alexsavio#9 but you might not agree or decide to invoke it differently (i.e. without growing number of travis runs) ;-)

ENH: travis -- add a run which would execute with DUECREDIT_ENABLE
@coveralls
Copy link

coveralls commented May 6, 2016

Coverage Status

Coverage decreased (-0.002%) to 72.475% when pulling 9d096f4 on alexsavio:enh/duecredit into 238fec1 on nipy:master.

@aweinstein
Copy link
Contributor

@satra I just did a PR to @alexsavio repository. This version merge master to this PR, and fix the bugs that were preventing it to pass the integration tests.

@coveralls
Copy link

coveralls commented Aug 4, 2016

Coverage Status

Coverage increased (+0.01%) to 72.306% when pulling 8dd8468 on alexsavio:enh/duecredit into 6d8efd7 on nipy:master.

@chrisgorgo chrisgorgo merged commit 0f2cb9f into nipy:master Sep 6, 2016
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.

7 participants