Merge pull request #1963 from effigies/ref/mris_expand

RF: Derive MRIsExpand from FSSurfaceCommand

Author: satra

nipype/interfaces/freesurfer/base.py

@@ -216,18 +216,19 @@ 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 == '':
             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.abspath(os.path.join(path, base))
+        return os.path.join(path, base)
 
 
 class FSScriptCommand(FSCommand):

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

nipype/interfaces/freesurfer/tests/test_auto_FSSurfaceCommand.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',

nipype/interfaces/freesurfer/tests/test_auto_MRIsExpand.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)

nipype/interfaces/freesurfer/tests/test_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.
     <https://surfer.nmr.mgh.harvard.edu/fswiki/mris_convert>`_
 
-    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 = './<filename>', 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 = './<filename>'``, 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)
 
@@ -3001,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'
@@ -3027,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',
@@ -3037,7 +3036,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 +3062,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 +3080,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)