ENH: initial support for duecredit #608
Conversation
fmriprep/__init__.py
Outdated
| @@ -44,5 +44,19 @@ def _new_version(): | |||
| iflogger.setLevel(level) | |||
| if v is None: | |||
| iflogger.warn('afni_vcheck executable not found') | |||
| return v | |||
There was a problem hiding this comment.
Guessing this was an accidental removal?
fmriprep/__init__.py
Outdated
| Info.version = staticmethod(_new_version) | ||
|
|
||
| from .due import due, Url |
There was a problem hiding this comment.
Could you move the import to the top? Just above warnings seems reasonable.
And I think we need to pull in the due.py stub for this to work. Alternately, drop the . and add duecredit to the dependencies.
There was a problem hiding this comment.
Very sorry @effigies for the rushed up and sloppy work I have done! I am usually better ;) Fixed up commit is coming shortly!
yarikoptic
force-pushed
the
enh-duecredit
branch
2 times, most recently
from
f53bfc0
to
2d287ce
Compare
fmriprep/workflows/epi.py
Outdated
| @@ -747,6 +748,11 @@ def _aslist(in_value): | |||
| return workflow | |||
|
|
|||
|
|
|||
| @due.cite( | |||
There was a problem hiding this comment.
jesus! I need a vacation!!!! pushed the fixed version
sorry again ;)
| @@ -131,6 +132,11 @@ def _pick1st(inlist): | |||
| # Helper functions | |||
| # ------------------------------------------------------ | |||
|
|
|||
| @due.cite( | |||
yarikoptic
force-pushed
the
enh-duecredit
branch
from
2d287ce
to
c8f8301
Compare
| Doi('10.1006/nimg.2001.1054'), | ||
| description="Conversion of the input phase-difference map into a fieldmap in Hz", | ||
| tags=['edu'] | ||
| ) |
There was a problem hiding this comment.
Looks like Function doesn't play well with decorated functions. I guess upgrading it to a proper interface would be the easiest solution.
There was a problem hiding this comment.
What is specific problem or Could you please share some easy way to reproduce the problem? (got lost a bit in that circle-ci setup to actually run the test(s))
There was a problem hiding this comment.
Sure:
from nipype.pipeline import engine as pe
from nipype.interfaces import utility as niu
@due.dcite(...)
def identfunc(x):
return x
fnode = pe.Node(niu.Function(function=identfunc), name='fnode')
fnode.inputs.x = 1
fnode.run()The specific problem is that nipype serializes the function as a string, and then reconstitutes it when it's time to run, which doesn't play nicely with decorators.
Update enh-duecredit and promote phdiff2fmap to interface
fmriprep/__init__.py
Outdated
| @@ -23,6 +23,7 @@ | |||
| __description__, | |||
| __longdesc__ | |||
| ) | |||
| from .due import due, Url | |||
There was a problem hiding this comment.
thanks! pushed the fix. btw -- feel welcome to push those tiny fixes directly into my fork/branch -- all devs of fmriprep should have permission for that
|
Huh. Apparently I can push to your branch. GitHub's permission structure is weird. Anyway, @yarikoptic, what's the to-do list here? |
Since a few months ago there's a checkbox you can use to disable permission to repo owners to push to the branch (in the PR initial request menu). |
|
I guess I would need to check on the laptop first since diff suggests some unrelated changes now... This PR was only for the basic references at that point, so the only Todo was to not cause the disturbance in normal operation of fmriprep ;-) |
|
|
||
| import warnings | ||
|
|
||
| # cmp is not used by fmriprep, so ignore nipype-generated warnings | ||
| warnings.filterwarnings('ignore', r'cmp not installed') | ||
|
|
||
| due.cite( | ||
| # Chicken/egg problem with Zenodo here regarding DOI. Might need |
There was a problem hiding this comment.
What is the status regarding this? There is a DOI for the Zenodo entry now, right?
| references_ = [{ | ||
| 'entry': Doi('10.1006/nimg.2001.1054'), | ||
| 'description': "Conversion of the input phase-difference map into a fieldmap in Hz", | ||
| 'tags': ['edu']}] |
|
I think we can address this on nipype proper, when we upstream #1024 from here. When upstreaming, we would make the integration with duecredit and add the feature that @satra proposed (having a flag to stop the visitor going deeper into subworkflows from a given point). Am I right, @yarikoptic, thinking that you would prefer a general implementation that solves the problem just once within Nipype rather than a custom solution for FMRIPREP (and others)? WDYT? Please reopen if you disagree/see it differently. |
|
Hi @oesteban ... sure thing IMHO the generic solution within nipype (if possible) would be preferable over adhoc in fmriprep ;-) |
Only partially addresses #600
There might be more citations in the code here and probably more to be added within niworkflows .
This use-case pointed (as I left a note in the code) to the chicken&egg problem. If DOI to be generated only upon the release, duecredit should support fetching citation directly from zenodo (duecredit/duecredit#117)