ENH: Allow transform to be saved from AFNI 3dWarp #2642
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2642 +/- ##
==========================================
- Coverage 67.62% 67.58% -0.04%
==========================================
Files 340 340
Lines 43003 43033 +30
Branches 5321 5327 +6
==========================================
+ Hits 29080 29086 +6
- Misses 13225 13249 +24
Partials 698 698
Continue to review full report at Codecov.
|
nipype/interfaces/afni/preprocess.py
Outdated
| @@ -2789,6 +2790,13 @@ class WarpInputSpec(AFNICommandInputSpec): | |||
| argstr='-zpad %d') | |||
| verbose = traits.Bool( | |||
| desc='Print out some information along the way.', argstr='-verb') | |||
| save_warp = traits.Bool( | |||
| False, usedefault=True, desc='save warp as .mat file') | |||
There was a problem hiding this comment.
The default of a Bool is already false. You can just have desc here.
There was a problem hiding this comment.
Oh, and RE your initial comment, I would set requires=['verbose']. I'm not sure what will happen if someone sets verbose=False, but at least it will catch when verbose is unset.
nipype/interfaces/afni/preprocess.py
Outdated
| warp_file = fname_presuffix(self.inputs.out_file, | ||
| suffix='_transform.mat', | ||
| use_ext=False) | ||
| np.savetxt(warp_file, [runtime.stdout], fmt=str('%s')) |
There was a problem hiding this comment.
Rather than calculate the filename twice:
if self.inputs.save_warp:
import numpy as np
warp_file = self._list_outputs()['warp_file']
np.savetxt(warp_file, [runtime.stdout], fmt='%s')(Not sure why you have str('%s'). If that's important, feel free to re-add.)
There was a problem hiding this comment.
(Not sure why you have str('%s'). If that's important, feel free to re-add.)
When I don't explicit the format, I have an error
TypeError: Mismatch between array dtype ('<U209') and format specifier ('%.18e')
So we need to keep it I think.
There was a problem hiding this comment.
Oh, I meant str('%s') vs '%s' (or is that also what you meant?). The str() seems like it should be a null-op, unless there's some weird Python2/3 hackery going on here.
There was a problem hiding this comment.
well on python 2.7 I am getting this error with fmt='%s'
ValueError: invalid fmt: u'%s'
nipype/interfaces/afni/preprocess.py
Outdated
|
|
||
| class WarpOutputSpec(TraitedSpec): | ||
| out_file = File(desc='Warped file.', exists=True) | ||
| warp_file = File(desc='warp transfrom .mat file') |
There was a problem hiding this comment.
what do you mean ?
There was a problem hiding this comment.
Oh, sorry. That's regular expression syntax. I just meant you misspelled "transform".
There was a problem hiding this comment.
Sorry ! I am correcting that
nipype/interfaces/afni/preprocess.py
Outdated
| use_ext=False) | ||
| outputs['warp_file'] = os.path.abspath(warp_file) | ||
|
|
||
| return outputs |
There was a problem hiding this comment.
Would the following work:
def _list_outputs(self):
outputs = super(Warp, self)._list_outputs()
if self.inputs.save_warp:
outputs['warp_file'] = fname_presuffix(outputs['out_file'],
suffix='_transform.mat',
use_ext=False)
return outputs
Ok. I was putting the WIP because I was unsure there isn't a better solution to force |
effigies
changed the title
|
Thanks. The test failures are related to #2650. |
Ok. Thank you for the review ! |
I added an option to save the warp from the stdout because it can be useful. But the warp transform is printed only if
verboseis set to True, otherwise the saved file is empty. I don't know if there is something like thexorandrequiresthat checks if a required trait is set to True.