-
Notifications
You must be signed in to change notification settings - Fork 530
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
FIX: ANTs' tools maintenance overhaul #3180
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3180 +/- ##
==========================================
- Coverage 64.88% 64.18% -0.70%
==========================================
Files 299 299
Lines 39506 39502 -4
Branches 5219 5219
==========================================
- Hits 25632 25355 -277
- Misses 12824 13110 +286
+ Partials 1050 1037 -13
Continue to review full report at Codecov.
|
df89399
to
dffcb78
Compare
I was waiting on CI to pass to read through. I'll look tonight or tomorrow morning. |
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.
Seems fine. Minor suggestions. Have you checked the generated docs?
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.
Actually, thought a bit about _list_outputs
, and then caught a couple other things.
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
5acff55
to
f88add4
Compare
Suggestions proposed on oesteban#10. |
RF: Move header copying to a mixin
@satra - thumbs up after Chris' suggestions? |
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.
FWIW
Having issues with Traceback (most recent call last):
File "/home/oesteban/workspace/nipype/nipype/pipeline/plugins/multiproc.py", line 67, in run_node
result["result"] = node.run(updatehash=updatehash)
File "/home/oesteban/workspace/nipype/nipype/pipeline/engine/nodes.py", line 516, in run
result = self._run_interface(execute=True)
File "/home/oesteban/workspace/nipype/nipype/pipeline/engine/nodes.py", line 635, in _run_interface
return self._run_command(execute)
File "/home/oesteban/workspace/nipype/nipype/pipeline/engine/nodes.py", line 741, in _run_command
result = self._interface.run(cwd=outdir)
File "/home/oesteban/workspace/nipype/nipype/interfaces/base/core.py", line 398, in run
runtime = self._post_run_hook(runtime)
File "/home/oesteban/workspace/nipype/nipype/interfaces/mixins/fixheader.py", line 132, in _post_run_hook
_copy_header(inputs[inp], outputs[out], keep_dtype=keep_dtype)
KeyError: 'bias_image' it seems the mixin is not general enough? |
Oh, I hadn't considered the output file might not exist. Yeah, I think a simple existence check makes sense. I think silent pass-through is fine for a mixin, as it's not responsible to make sure the file exists, only that if the file exists it has the right header. |
In the last maintenance (nipy#3180) of the interface, we eliminated an important section of the ``_list_outputs``: nipy@6979cbd#diff-b6f33a19b0e06b91023416db5faf7323L544-L547 This PR addresses the problem: ``` Execution Outputs ----------------- * bias_image : <undefined> * output_image : /home/oesteban/tmp/fmriprep-ds005/fprep-work/fmriprep_wf/single_subject_01_wf/anat_preproc_wf/brain_extraction_wf/inu_n4_final/mapflow/_inu_n4_final0/sub-01_T1w_cor rected.nii.gz Runtime info ------------ * cmdline : N4BiasFieldCorrection --bspline-fitting [ 200 ] -d 3 --input-image /oak/stanford/groups/russpold/data/openfmri/ds000005/sub-01/anat/sub-01_T1w.nii.gz --convergence [ 50x 50x50x50x50, 1e-07 ] --output [ sub-01_T1w_corrected.nii.gz, sub-01_T1w_bias.nii.gz ] -r --shrink-factor 4 --weight-image /home/oesteban/tmp/fmriprep-ds005/fprep-work/fmriprep_wf/single_subject_01_wf/anat_preproc_wf/brain_extraction_wf/atropos_wf/copy_xform/09_relabel_wm_maths_xform.nii.gz * duration : 15.334786 * hostname : dendrite * prev_wd : /home/oesteban/tmp/fmriprep-ds005 * working_dir : /home/oesteban/tmp/fmriprep-ds005/fprep-work/fmriprep_wf/single_subject_01_wf/anat_preproc_wf/brain_extraction_wf/inu_n4_final/mapflow/_inu_n4_final0 ``` (`bias_image` should be `'sub-01_T1w_bias.nii.gz'` given the `cmdline`)
This PR:
Atropos
(interfaces.ants.segmentation does not have options for PriorLabelImage arguments #3178)JointFusion
interface, for which there hasn't been any corresponding command line binary for very long time now.