Afni workshop #2135
Afni workshop #2135
Conversation
nipype/interfaces/afni/__init__.py
Outdated
| @@ -8,7 +8,8 @@ | |||
| """ | |||
|
|
|||
| from .base import Info | |||
| from .preprocess import (Allineate, Automask, AutoTcorrelate, | |||
| from .preprocess import (AlignEpiAnatPy,Allineate, Automask, | |||
| AutoTcorrelate,AutoTLRC, | |||
There was a problem hiding this comment.
Style point: spaces after commas.
nipype/interfaces/afni/preprocess.py
Outdated
| #default='/opt/miniconda/envs/py27/bin/python', | ||
| mandatory=True, | ||
| position=0 | ||
| ) |
There was a problem hiding this comment.
A couple points:
Shouldn't align_epi_anat.py be an executable in the path with #!/usr/bin/env python27 or similar? If this is really necessary, how about:
py27_path = traits.Either('python2', File(exists=True), usedefault=True, ...)Further, it might make sense to abstract some of this Python stuff to:
class AFNIPythonCommandInputSpec(AFNICommandInputSpec):
py27_path = traits.Either('python2', File(exists=True), usedefault=True, ...)
class AFNIPythonCommand(AFNICommand):
@property
def cmd(self):
return spawn.find_executable(super(AFNIPythonCommand, self).cmd)
@property
def cmdline(self):
return "{} {}".format(self.inputs.py27_path, super(AFNIPythonCommand, self).cmdline)Then you can write the interface consistently with non-Python interfaces.
class AlignEpiAnatPy(AFNIPythonCommand):
_cmd = 'align_epi_anat.py'
....
nipype/interfaces/afni/preprocess.py
Outdated
| desc="skull-stripped (not aligned) volume") | ||
|
|
||
| class AlignEpiAnatPy(AFNICommand): | ||
| """align EPI to anatomical datasets or vice versa |
nipype/interfaces/afni/preprocess.py
Outdated
| desc='the epi base used in alignment' | ||
| 'should be one of (0/mean/median/max/subbrick#)', | ||
| mandatory=True, | ||
| argstr='-epi_base %s') |
There was a problem hiding this comment.
Looks like this is an integer with minimum of 0, or one of the three strings "mean", "median" or "max"?
epi_base = traits.Either(traits.Range(low=0), traits.Enum('mean', 'median', 'max'), ...)
nipype/interfaces/afni/preprocess.py
Outdated
| anat2epi = traits.Bool( | ||
| default = True, | ||
| desc='align anatomical to EPI dataset (default)', | ||
| argstr='-anat2epi') |
There was a problem hiding this comment.
The "default = True" here won't have any effect, because you aren't using usedefault=True. The following is probably what you want:
anat2epi = traits.Bool(True, usedefault=True, desc='...', argstr='-anat2epi')Given that it is the default behavior, it won't make a semantic difference, though...
nipype/interfaces/afni/preprocess.py
Outdated
| if not isdefined(self.inputs.suffix): | ||
| suffix = '_al' | ||
| else: | ||
| suffix = '_'+self.inputs.suffix |
There was a problem hiding this comment.
With the default set as 'al' above, you can replace this if/else with:
suffix = '_' + self.inputs.suffix
nipype/interfaces/afni/preprocess.py
Outdated
| else: | ||
| suffix = '_'+self.inputs.suffix | ||
| if self.inputs.anat2epi: | ||
| outputs['anat_al_orig'] = os.path.abspath(self._gen_fname(anat_prefix, suffix=suffix+'+orig')+ext) |
There was a problem hiding this comment.
AFNICommand._gen_fname already prepends the cwd, so you can drop the os.path.abspath. I'm a little confused by the ext here. It looks like, if outputtype == 'NIFTI', the filename will end with _al+orig.nii.HEAD. I'm not very familiar with AFNI's formats, but I'd assume .HEADand.nii` are mutually exclusive.
Perhaps what you want is:
outputtype = self.inputs.outputtype
ext = '.HEAD' if outputtype == 'AFNI' else Info.output_type_to_ext(outputtype)
[...]
outputs['anat_al_orig'] = self._gen_fname(anat_prefix, suffix=suffix + '+orig', ext=ext)And so-on for the rest.
| out_file = File( | ||
| desc='collect output to a file', | ||
| argstr=' |& tee %s', | ||
| position=-1) |
There was a problem hiding this comment.
This may be reimplementing terminal_output, though that splits stderr and stdout.
nipype/interfaces/afni/utils.py
Outdated
| traits.Tuple( | ||
| (File( | ||
| exists=True, | ||
| desc='input file', |
There was a problem hiding this comment.
I don't think desc makes a difference in internal traits.
| @@ -491,6 +701,76 @@ class Copy(AFNICommand): | |||
| input_spec = CopyInputSpec | |||
| output_spec = AFNICommandOutputSpec | |||
|
|
|||
| class DotInputSpec(AFNICommandInputSpec): | |||
| in_files = traits.List( | |||
| (File()), | |||
There was a problem hiding this comment.
File(exists=True), presumably?
There was a problem hiding this comment.
The check for file existence isn't currently compatible with AFNI's sub-brick selector, until I get around to addressing that, I'd like to leave off the existence requirement for this interface.
Codecov Report
@@ Coverage Diff @@
## master #2135 +/- ##
==========================================
+ Coverage 72.19% 72.23% +0.04%
==========================================
Files 1158 1167 +9
Lines 58032 58344 +312
Branches 8336 8388 +52
==========================================
+ Hits 41895 42147 +252
- Misses 14815 14866 +51
- Partials 1322 1331 +9
Continue to review full report at Codecov.
|
|
hmm, it seems like there's not a reliable place to grab the python 2 executable across all the travis builds. @effigies Do you have any ideas how I could structure the interfaces for the AFNI python scripts to cope with this? |
|
@Shotgunosine - perhaps we should do the same thing we did for the nimh workshop and have both environments available. |
|
@satra would the best way to do that be to modify the .travis.yml? |
|
both travis and the docker build process for circle. |
|
Looks like I misdiagnosed the issue with respect to Travis at least. On the Travis builds without AFNI installed 'distutils.spawn' couldn't find the executable and so it was returning |
|
Hmm, I'm not sure I understand the CircleCI error. I've copied the Traceback below, does that give anyone any insight into what the issue might be? |
|
That issue should be resolved by #2137, which occurred after your fork. Merge or rebase against master to get that update. |
Shotgunosine
force-pushed
the
afni_workshop
branch
from
d1f0b3e to
1254589
Compare
|
Now CircleCI is dying earlier with the following error: |
|
Restarted the build. Looks like it made it through, this time. |
adding minimal interface wrappers needed to wrap a basic AFNI functional processing pipeline