Afni nwarpadjust #2450
Afni nwarpadjust #2450
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2450 +/- ##
==========================================
- Coverage 66.86% 66.86% -0.01%
==========================================
Files 328 328
Lines 42560 42586 +26
Branches 5282 5287 +5
==========================================
+ Hits 28457 28474 +17
- Misses 13405 13412 +7
- Partials 698 700 +2
Continue to review full report at Codecov.
|
nipype/interfaces/afni/utils.py
Outdated
| name_source='in_files', | ||
| name_template='%s_NwarpAdjust', | ||
| keep_extension=True, | ||
| xand=['in_files']) |
There was a problem hiding this comment.
you can remove this since in_files is mandatory. also there is no xand in the current spec. so make specs would have returned a violation.
There was a problem hiding this comment.
actually I am trying to make in_files not mandatory, because what is needed here is only the warps
There was a problem hiding this comment.
I believe what you want is requires=['in_files']. See https://nipype.readthedocs.io/en/latest/devel/interface_specs.html.
nipype/interfaces/afni/utils.py
Outdated
| 'The output dataset will be on the common grid shared by the ' | ||
| 'source datasets.', | ||
| argstr='-prefix %s', | ||
| mandatory=False, |
There was a problem hiding this comment.
here and in the previous input field mandatory=False is not necessary.
| input_spec = NwarpAdjustInputSpec | ||
| output_spec = AFNICommandOutputSpec | ||
|
|
||
| def _parse_inputs(self, skip=None): |
There was a problem hiding this comment.
i think this function can be skipped as well.
|
Hi @salma1601 do you have some time to address @satra's comments? If you have any questions, please let us know. |
|
@effigies when I remove the |
|
Please merge master to fix the tests. I'm not very familiar with |
There was a problem hiding this comment.
hi @salma1601, I think we can reduce the amount of boilerplate on this interface. Could you try out the suggested changes and see if things are still running okay?
nipype/interfaces/afni/utils.py
Outdated
| @@ -1516,6 +1516,90 @@ def _list_outputs(self): | |||
| return outputs | |||
|
|
|||
|
|
|||
| class NwarpAdjustInputSpec(CommandLineInputSpec): | |||
nipype/interfaces/afni/utils.py
Outdated
| skip += ['out_file'] | ||
| return super(NwarpAdjust, self)._parse_inputs(skip=skip) | ||
|
|
||
| def _gen_filename(self, name): |
There was a problem hiding this comment.
this probably isn't necessary either since you're calling name_source in the inputspec
nipype/interfaces/afni/utils.py
Outdated
|
|
||
| def _list_outputs(self): | ||
| outputs = self.output_spec().get() | ||
|
|
There was a problem hiding this comment.
perhaps this can be cleaned up to just
...
if self.inputs.in_files:
outputs['out_file'] = os.path.abspath(self.inputs.out_file)
return outputsThere was a problem hiding this comment.
it doesn't work when I don't give exactly how to build outputs['out_file'] from in_files: the command line is correct but it fails at returning the outputs and I have an error
salma1601
force-pushed
the
afni_nwarpadjust
branch
from
aef99f0 to
3a27625
Compare
Yes I tried, the command line is wrong without _parse_inputs |
|
@satra I think @salma1601 just discovered a bug here: if a trait with |
|
@oesteban - yes we should track this, but since the |
|
maybe the ordering: filling the |
|
Actually, the |
|
we should figure out the logic here. if requires is present then setting or using out_file to generate out_file needs in_files. otherwise the logic here is stating, if out_file use out_file perhaps in this scenario, there should be a default |
|
Two thumbs up, so in it goes. I'll create an issue for 1.0.3. |
add interface for AFNI 3dNwarpAdjust