From 1437dc48b3ed19b73ab2e8f0f14f64cfa2f9376a Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 21 Apr 2017 09:32:14 -0400 Subject: [PATCH 1/5] RF: Derive MRIsExpand from FSSurfaceCommand --- nipype/interfaces/freesurfer/base.py | 7 ++++--- nipype/interfaces/freesurfer/utils.py | 23 ++++------------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/nipype/interfaces/freesurfer/base.py b/nipype/interfaces/freesurfer/base.py index c07a1775fa..c3107b299c 100644 --- a/nipype/interfaces/freesurfer/base.py +++ b/nipype/interfaces/freesurfer/base.py @@ -216,10 +216,11 @@ def _normalize_filenames(self): def _associated_file(in_file, out_name): """Based on MRIsBuildFileName in freesurfer/utils/mrisurf.c + If no path information is provided for out_name, use path and + hemisphere (if also unspecified) from in_file to determine the path + of the associated file. Use in_file prefix to indicate hemisphere for out_name, rather than inspecting the surface data structure. - Also, output to in_file's directory if path information not provided - for out_name. """ path, base = os.path.split(out_name) if path == '': @@ -227,7 +228,7 @@ def _associated_file(in_file, out_name): hemis = ('lh.', 'rh.') if in_file[:3] in hemis and base[:3] not in hemis: base = in_file[:3] + base - return os.path.abspath(os.path.join(path, base)) + return os.path.join(path, base) class FSScriptCommand(FSCommand): diff --git a/nipype/interfaces/freesurfer/utils.py b/nipype/interfaces/freesurfer/utils.py index 6db3337058..9f5a32b2cd 100644 --- a/nipype/interfaces/freesurfer/utils.py +++ b/nipype/interfaces/freesurfer/utils.py @@ -3037,7 +3037,7 @@ class MRIsExpandOutputSpec(TraitedSpec): out_file = File(desc='Output surface file') -class MRIsExpand(FSCommand): +class MRIsExpand(FSSurfaceCommand): """ Expands a surface (typically ?h.white) outwards while maintaining smoothness and self-intersection constraints. @@ -3063,7 +3063,9 @@ def _list_outputs(self): self.inputs.out_name) return outputs - def _get_filecopy_info(self): + def _normalize_filenames(self): + """ Find full paths for pial, thickness and sphere files for copying + """ in_file = self.inputs.in_file pial = self.inputs.pial @@ -3079,20 +3081,3 @@ def _get_filecopy_info(self): thickness_name) self.inputs.sphere = self._associated_file(in_file, self.inputs.sphere) - - return super(MRIsExpand, self)._get_filecopy_info() - - @staticmethod - def _associated_file(in_file, out_name): - """Based on MRIsBuildFileName in freesurfer/utils/mrisurf.c - - Use file prefix to indicate hemisphere, rather than inspecting the - surface data structure - """ - path, base = os.path.split(out_name) - if path == '': - path, in_file = os.path.split(in_file) - hemis = ('lh.', 'rh.') - if in_file[:3] in hemis and base[:3] not in hemis: - base = in_file[:3] + base - return os.path.join(path, base) From 3bd1cf0b4bb8f6b27e556155f8d35b4f1aae8e85 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 21 Apr 2017 09:42:42 -0400 Subject: [PATCH 2/5] RF/DOC: Simplify and document MRIsCombine slightly more --- nipype/interfaces/freesurfer/utils.py | 37 +++++++++++++-------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/nipype/interfaces/freesurfer/utils.py b/nipype/interfaces/freesurfer/utils.py index 9f5a32b2cd..544fe74f3f 100644 --- a/nipype/interfaces/freesurfer/utils.py +++ b/nipype/interfaces/freesurfer/utils.py @@ -976,16 +976,18 @@ class MRIsCombineOutputSpec(TraitedSpec): class MRIsCombine(FSSurfaceCommand): """ - Uses Freesurfer's mris_convert to combine two surface files into one. + Uses Freesurfer's ``mris_convert`` to combine two surface files into one. For complete details, see the `mris_convert Documentation. `_ - If given an out_file that does not begin with 'lh.' or 'rh.', - mris_convert will prepend 'lh.' to the file name. - To avoid this behavior, consider setting out_file = './', or + If given an ``out_file`` that does not begin with ``'lh.'`` or ``'rh.'``, + ``mris_convert`` will prepend ``'lh.'`` to the file name. + To avoid this behavior, consider setting ``out_file = './'``, or leaving out_file blank. + In a Node/Workflow, ``out_file`` is interpreted literally. + Example ------- @@ -1003,24 +1005,21 @@ class MRIsCombine(FSSurfaceCommand): def _list_outputs(self): outputs = self._outputs().get() - outputs['out_file'] = self._associated_file(self.inputs.in_files[0], - self.inputs.out_file) - return outputs - @staticmethod - def _associated_file(in_file, out_name): - """Unlike the standard _associated_file, which uses the prefix from - in_file, in MRIsCombine, it uses 'lh.' as the prefix for the output - file no matter what the inputs are. - """ - path, base = os.path.split(out_name) - if path == '': - hemis = ('lh.', 'rh.') - if base[:3] not in hemis: - base = 'lh.' + base - return os.path.abspath(os.path.join(path, base)) + # mris_convert --combinesurfs uses lh. as the default prefix + # regardless of input file names, except when path info is + # specified + path, base = os.path.split(self.inputs.out_file) + if path == '' and base[:3] not in ('lh.', 'rh.'): + base = 'lh.' + base + outputs['out_file'] = os.path.abspath(os.path.join(path, base)) + + return outputs def _normalize_filenames(self): + """ In a Node context, interpret out_file as a literal path to + reduce surprise. + """ if isdefined(self.inputs.out_file): self.inputs.out_file = os.path.abspath(self.inputs.out_file) From e9af42c6b59b7453b4e38e7f123962121b9b4f42 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 21 Apr 2017 11:03:19 -0400 Subject: [PATCH 3/5] Add manual FSSurfaceCommand tests --- .../freesurfer/tests/test_FSSurfaceCommand.py | 50 +++++++++++++++++++ .../tests/test_auto_FSSurfaceCommand.py | 24 --------- 2 files changed, 50 insertions(+), 24 deletions(-) create mode 100644 nipype/interfaces/freesurfer/tests/test_FSSurfaceCommand.py delete mode 100644 nipype/interfaces/freesurfer/tests/test_auto_FSSurfaceCommand.py diff --git a/nipype/interfaces/freesurfer/tests/test_FSSurfaceCommand.py b/nipype/interfaces/freesurfer/tests/test_FSSurfaceCommand.py new file mode 100644 index 0000000000..acaa5d466d --- /dev/null +++ b/nipype/interfaces/freesurfer/tests/test_FSSurfaceCommand.py @@ -0,0 +1,50 @@ +# AUTO-GENERATED by tools/checkspecs.py on 2017.04.21 +# Modified 2017.04.21 by Chris Markiewicz +from __future__ import unicode_literals +import pytest + +from ..base import FSSurfaceCommand +from ... import freesurfer as fs +from ...io import FreeSurferSource + + +def test_FSSurfaceCommand_inputs(): + input_map = dict(args=dict(argstr='%s', + ), + environ=dict(nohash=True, + usedefault=True, + ), + ignore_exception=dict(nohash=True, + usedefault=True, + ), + subjects_dir=dict(), + terminal_output=dict(nohash=True, + ), + ) + inputs = FSSurfaceCommand.input_spec() + + for key, metadata in list(input_map.items()): + for metakey, value in list(metadata.items()): + assert getattr(inputs.traits()[key], metakey) == value + + +@pytest.mark.skipif(fs.no_freesurfer(), reason="freesurfer is not installed") +def test_associated_file(): + fssrc = FreeSurferSource(subjects_dir=fs.Info.subjectsdir(), + subject_id='fsaverage', hemi='lh') + + fsavginfo = fssrc.run().outputs.get() + + # Pairs of white/pial files in the same directories + for white, pial in [('lh.white', 'lh.pial'), + ('./lh.white', './lh.pial'), + (fsavginfo['white'], fsavginfo['pial'])]: + + # Unspecified paths, possibly with missing hemisphere information, + # are equivalent to using the same directory and hemisphere + for name in ('pial', 'lh.pial', pial): + assert FSSurfaceCommand._associated_file(white, name) == pial + + # With path information, no changes are made + for name in ('./pial', './lh.pial', fsavginfo['pial']): + assert FSSurfaceCommand._associated_file(white, name) == name diff --git a/nipype/interfaces/freesurfer/tests/test_auto_FSSurfaceCommand.py b/nipype/interfaces/freesurfer/tests/test_auto_FSSurfaceCommand.py deleted file mode 100644 index 0c3fcc8984..0000000000 --- a/nipype/interfaces/freesurfer/tests/test_auto_FSSurfaceCommand.py +++ /dev/null @@ -1,24 +0,0 @@ -# AUTO-GENERATED by tools/checkspecs.py - DO NOT EDIT -from __future__ import unicode_literals -from ..base import FSSurfaceCommand - - -def test_FSSurfaceCommand_inputs(): - input_map = dict(args=dict(argstr='%s', - ), - environ=dict(nohash=True, - usedefault=True, - ), - ignore_exception=dict(nohash=True, - usedefault=True, - ), - subjects_dir=dict(), - terminal_output=dict(nohash=True, - ), - ) - inputs = FSSurfaceCommand.input_spec() - - for key, metadata in list(input_map.items()): - for metakey, value in list(metadata.items()): - assert getattr(inputs.traits()[key], metakey) == value - From 4b6968f6963498a9d889c2c4b9f26e985b360cf3 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 21 Apr 2017 12:20:42 -0400 Subject: [PATCH 4/5] TEST: Test that MRIsExpand runs correctly --- .../interfaces/freesurfer/tests/test_utils.py | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/nipype/interfaces/freesurfer/tests/test_utils.py b/nipype/interfaces/freesurfer/tests/test_utils.py index eff2946000..8e756ab401 100644 --- a/nipype/interfaces/freesurfer/tests/test_utils.py +++ b/nipype/interfaces/freesurfer/tests/test_utils.py @@ -8,8 +8,10 @@ from nipype.testing.fixtures import (create_files_in_directory_plus_dummy_file, create_surf_file_in_directory) +from nipype.pipeline import engine as pe +from nipype.interfaces import freesurfer as fs from nipype.interfaces.base import TraitError -import nipype.interfaces.freesurfer as fs +from nipype.interfaces.io import FreeSurferSource @pytest.mark.skipif(fs.no_freesurfer(), reason="freesurfer is not installed") @@ -159,3 +161,52 @@ def test_surfshots(create_files_in_directory_plus_dummy_file): os.environ["DISPLAY"] = hold_display except KeyError: pass + + +@pytest.mark.skipif(fs.no_freesurfer(), reason="freesurfer is not installed") +def test_mrisexpand(tmpdir): + fssrc = FreeSurferSource(subjects_dir=fs.Info.subjectsdir(), + subject_id='fsaverage', hemi='lh') + + fsavginfo = fssrc.run().outputs.get() + + # dt=60 to ensure very short runtime + expand_if = fs.MRIsExpand(in_file=fsavginfo['smoothwm'], + out_name='expandtmp', + distance=1, + dt=60) + + expand_nd = pe.Node( + fs.MRIsExpand(in_file=fsavginfo['smoothwm'], + out_name='expandtmp', + distance=1, + dt=60), + name='expand_node') + + # Interfaces should have same command line at instantiation + orig_cmdline = 'mris_expand -T 60 {} 1 expandtmp'.format(fsavginfo['smoothwm']) + assert expand_if.cmdline == orig_cmdline + assert expand_nd.interface.cmdline == orig_cmdline + + # Run both interfaces + if_res = expand_if.run() + nd_res = expand_nd.run() + + # Commandlines differ + node_cmdline = 'mris_expand -T 60 -pial {cwd}/lh.pial {cwd}/lh.smoothwm ' \ + '1 expandtmp'.format(cwd=nd_res.runtime.cwd) + assert if_res.runtime.cmdline == orig_cmdline + assert nd_res.runtime.cmdline == node_cmdline + + # Check output + if_out_file = if_res.outputs.get()['out_file'] + nd_out_file = nd_res.outputs.get()['out_file'] + # Same filename + assert op.basename(if_out_file) == op.basename(nd_out_file) + # Interface places output in source directory + assert op.dirname(if_out_file) == op.dirname(fsavginfo['smoothwm']) + # Node places output in working directory + assert op.dirname(nd_out_file) == nd_res.runtime.cwd + + # Remove test surface + os.unlink(if_out_file) From 5da86ea832392a8f4898b19e3c23155b4105916b Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 21 Apr 2017 12:21:26 -0400 Subject: [PATCH 5/5] FIX: Remove dev option from interface until release --- .../freesurfer/tests/test_auto_MRIsExpand.py | 2 -- nipype/interfaces/freesurfer/utils.py | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/nipype/interfaces/freesurfer/tests/test_auto_MRIsExpand.py b/nipype/interfaces/freesurfer/tests/test_auto_MRIsExpand.py index cde82e2bd3..c74f31bd59 100644 --- a/nipype/interfaces/freesurfer/tests/test_auto_MRIsExpand.py +++ b/nipype/interfaces/freesurfer/tests/test_auto_MRIsExpand.py @@ -23,8 +23,6 @@ def test_MRIsExpand_inputs(): mandatory=True, position=-3, ), - navgs=dict(argstr='-navgs %d %d', - ), nsurfaces=dict(argstr='-N %d', ), out_name=dict(argstr='%s', diff --git a/nipype/interfaces/freesurfer/utils.py b/nipype/interfaces/freesurfer/utils.py index 544fe74f3f..85c2bf6779 100644 --- a/nipype/interfaces/freesurfer/utils.py +++ b/nipype/interfaces/freesurfer/utils.py @@ -3000,11 +3000,6 @@ class MRIsExpandInputSpec(FSTraitedSpec): desc=('Name of thickness file (implicit: "thickness")\n' 'If no path, uses directory of `in_file`\n' 'If no path AND missing "lh." or "rh.", derive from `in_file`')) - navgs = traits.Tuple( - traits.Int, traits.Int, - argstr='-navgs %d %d', - desc=('Tuple of (n_averages, min_averages) parameters ' - '(implicit: (16, 0))')) pial = traits.Str( argstr='-pial %s', copyfile=False, desc=('Name of pial file (implicit: "pial")\n' @@ -3026,6 +3021,11 @@ class MRIsExpandInputSpec(FSTraitedSpec): desc='Number of surfacces to write during expansion') # # Requires dev version - Re-add when min_ver/max_ver support this # # https://github.com/freesurfer/freesurfer/blob/9730cb9/mris_expand/mris_expand.c + # navgs = traits.Tuple( + # traits.Int, traits.Int, + # argstr='-navgs %d %d', + # desc=('Tuple of (n_averages, min_averages) parameters ' + # '(implicit: (16, 0))')) # target_intensity = traits.Tuple( # traits.Float, traits.File(exists=True), # argstr='-intensity %g %s',