Repository: nipy/nipype

Issue:
issues with afni.allineate w/ solution (and possible issue with genfile parameter)
###  Summary

Hi all. Just started learning nipype, and it's been pretty great so far. I've only run into issues with the `afni.allineate` interface. I was able to fix it on my own, but I thought I should post an issue here for awareness (plus I'm not sure how to do a pull request...)

There are 2 problems I've encountered. The first is that `afni.allineate` consistently fails the hash check when placed in a workflow and always reruns. I think this is because the `hash_files` parameter needs to be set to `False`.

```
out_file = File(
    desc='output file from 3dAllineate',
    argstr='-prefix %s',
    genfile=True,
    hash_files=False, # <-- when this is added it starts working again
    xor=['allcostx']) 
```
Referencing the docs, I think this is an appropriate change:
> `hash_files`
To be used with inputs that are defining output filenames. When this flag is set to false any Nipype will not try to hash any files described by this input. This is useful to avoid rerunning when the specified output file already exists and has changed.

The second issue is that the prefix (`out_file`) argument does not get generated when used in a workflow. This issue is also referenced here: #2216, but with a temporary workaround.

My understanding is that when `genfile` is set to `True`, the `_gen_filename()` method for the interface gets called when the user does not specify a value. Looking at the code, the `_gen_filename()` does seems to implemented, but not correctly:

```
def _gen_filename(self, name): # this gets called when `out_file` is not defined
    if name == 'out_file':
        return self._list_outputs()[name] # <-- we are returning the value of `out_file`, which is undefined...
    return None
```

So I changed it to this:

```
def _gen_filename(self, name): # this gets called when `out_file` is not defined
    if name == 'out_file':
        return self._gen_fname(self.inputs.in_file,op.dirname(self.inputs.in_file),suffix='_allineate')
    return None
```

Which should define the `out_file` to be in the same location and have the same name as the `in_file`, but with the '_allineate' suffix.

When I run this, the problem seems to be fixed, as the prefix is now defined:

```
[Node] Running "3dallineate_orig" ("nipype.interfaces.afni.preprocess.Allineate"), a CommandLine Interface with command:
3dAllineate -source /home/vana/Projects/p3/tmp/P3/_subject_id_sub-CTS200/3dallineate_orig/orig_out.nii.gz -prefix /home/vana/Projects/p3/tmp/P3/_subject_id_sub-CTS200/3dallineate_orig/orig_out_allineate.nii.gz -1Dmatrix_save FSorig.XFM.FS2MPR.aff12.1D -overwrite -base /home/vana/Projects/p3/dataset/sub-CTS200/anat/sub-CTS200_T1w.nii.gz
```
But downstream nodes don't seem to notice that the `out_file` is actually defined now... (Note: I have  a node whose in_file is connected to the out_file of the allineate node)

```
 [Node] Error on "P3.3drefit1" (/home/vana/Projects/p3/tmp/P3/_subject_id_sub-CTS200/3drefit1)
Traceback (most recent call last):
  File "./preproc.py", line 293, in <module>
    wf.run()
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/workflows.py", line 602, in run
    runner.run(execgraph, updatehash=updatehash, config=self.config)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/plugins/linear.py", line 44, in run
    node.run(updatehash=updatehash)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 487, in run
    result = self._run_interface(execute=True)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 571, in _run_interface
    return self._run_command(execute)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 638, in _run_command
    cmd = self._interface.cmdline
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 935, in cmdline
    self._check_mandatory_inputs()
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 389, in _check_mandatory_inputs
    raise ValueError(msg)
ValueError: Refit requires a value for input 'in_file'. For a list of required inputs, see Refit.help()
```

After looking at some of the fsl interfaces (and some trial-and-error), I figured out that the downstream nodes seem to be getting the information for their inputs from the `_list_outputs` method of the upstream node, and ignore `_gen_filename`. So when I do this:

```
def _gen_outfilename(self):
    out_file = self.inputs.out_file
    if not isdefined(out_file) and isdefined(self.inputs.in_file) and not isdefined(self.inputs.allcostx):
        out_file = op.abspath(self._gen_fname(self.inputs.in_file,op.dirname(self.inputs.in_file),suffix='_allineate'))
    return out_file

def _list_outputs(self):
    outputs = self.output_spec().get()

    outputs['out_file'] = self._gen_outfilename()

    # other stuff below this ...

def _gen_filename(self, name):
    if name == 'out_file':
        return self._gen_outfilename()
    return None
```

Everything works! But then the question becomes, what's the point of the `genfile` parameter if I have to define a method that checks if the input is defined anyway?

I'm not sure how to do a pull request to this repo, so I've attached my git patch below. (This is on nipype 1.0.1)
[allineate.patch.tar.gz](https://github.com/nipy/nipype/files/1826031/allineate.patch.tar.gz)

Discussion:

Patch:
diff --git a/nipype/interfaces/afni/preprocess.py b/nipype/interfaces/afni/preprocess.py
index 13a065c27..9ede8d13e 100644
--- a/nipype/interfaces/afni/preprocess.py
+++ b/nipype/interfaces/afni/preprocess.py
@@ -218,7 +218,9 @@ class AllineateInputSpec(AFNICommandInputSpec):
     out_file = File(
         desc='output file from 3dAllineate',
         argstr='-prefix %s',
-        genfile=True,
+        name_template='%s_allineate',
+        name_source='in_file',
+        hash_files=False,
         xor=['allcostx'])
     out_param_file = File(
         argstr='-1Dparam_save %s',
@@ -424,11 +426,11 @@ class AllineateInputSpec(AFNICommandInputSpec):
     _dirs = ['X', 'Y', 'Z', 'I', 'J', 'K']
     nwarp_fixmot = traits.List(
         traits.Enum(*_dirs),
-        argstr='-nwarp_fixmot%s',
+        argstr='-nwarp_fixmot%s...',
         desc='To fix motion along directions.')
     nwarp_fixdep = traits.List(
         traits.Enum(*_dirs),
-        argstr='-nwarp_fixdep%s',
+        argstr='-nwarp_fixdep%s...',
         desc='To fix non-linear warp dependency along directions.')
     verbose = traits.Bool(
         argstr='-verb', desc='Print out verbose progress reports.')
@@ -465,7 +467,6 @@ class Allineate(AFNICommand):
     '3dAllineate -source functional.nii -prefix functional_allineate.nii -1Dmatrix_apply cmatrix.mat'
     >>> res = allineate.run()  # doctest: +SKIP
 
-    >>> from nipype.interfaces import afni
     >>> allineate = afni.Allineate()
     >>> allineate.inputs.in_file = 'functional.nii'
     >>> allineate.inputs.reference = 'structural.nii'
@@ -473,23 +474,22 @@ class Allineate(AFNICommand):
     >>> allineate.cmdline
     '3dAllineate -source functional.nii -base structural.nii -allcostx |& tee out.allcostX.txt'
     >>> res = allineate.run()  # doctest: +SKIP
+
+    >>> allineate = afni.Allineate()
+    >>> allineate.inputs.in_file = 'functional.nii'
+    >>> allineate.inputs.reference = 'structural.nii'
+    >>> allineate.inputs.nwarp_fixmot = ['X', 'Y']
+    >>> allineate.cmdline
+    '3dAllineate -source functional.nii -nwarp_fixmotX -nwarp_fixmotY -prefix functional_allineate -base structural.nii'
+    >>> res = allineate.run()  # doctest: +SKIP
     """
 
     _cmd = '3dAllineate'
     input_spec = AllineateInputSpec
     output_spec = AllineateOutputSpec
 
-    def _format_arg(self, name, trait_spec, value):
-        if name == 'nwarp_fixmot' or name == 'nwarp_fixdep':
-            arg = ' '.join([trait_spec.argstr % v for v in value])
-            return arg
-        return super(Allineate, self)._format_arg(name, trait_spec, value)
-
     def _list_outputs(self):
-        outputs = self.output_spec().get()
-
-        if self.inputs.out_file:
-            outputs['out_file'] = op.abspath(self.inputs.out_file)
+        outputs = super(Allineate, self)._list_outputs()
 
         if self.inputs.out_weight_file:
             outputs['out_weight_file'] = op.abspath(
@@ -512,16 +512,10 @@ class Allineate(AFNICommand):
                 outputs['out_param_file'] = op.abspath(
                     self.inputs.out_param_file)
 
-        if isdefined(self.inputs.allcostx):
-            outputs['allcostX'] = os.path.abspath(
-                os.path.join(os.getcwd(), self.inputs.allcostx))
+        if self.inputs.allcostx:
+            outputs['allcostX'] = os.path.abspath(self.inputs.allcostx)
         return outputs
 
-    def _gen_filename(self, name):
-        if name == 'out_file':
-            return self._list_outputs()[name]
-        return None
-
 
 class AutoTcorrelateInputSpec(AFNICommandInputSpec):
     in_file = File(
diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py
index c199af3ca..d15628fd6 100644
--- a/nipype/interfaces/base/core.py
+++ b/nipype/interfaces/base/core.py
@@ -1072,6 +1072,12 @@ class CommandLine(BaseInterface):
         if not isdefined(retval) or "%s" in retval:
             if not trait_spec.name_source:
                 return retval
+
+            # Do not generate filename when excluded by other inputs
+            if trait_spec.xor and any(isdefined(getattr(self.inputs, field))
+                                      for field in trait_spec.xor):
+                return retval
+
             if isdefined(retval) and "%s" in retval:
                 name_template = retval
             else:


Test patch:
diff --git a/nipype/interfaces/afni/tests/test_auto_Allineate.py b/nipype/interfaces/afni/tests/test_auto_Allineate.py
index 59b1929fb..1c5476a42 100644
--- a/nipype/interfaces/afni/tests/test_auto_Allineate.py
+++ b/nipype/interfaces/afni/tests/test_auto_Allineate.py
@@ -61,12 +61,14 @@ def test_Allineate_inputs():
             usedefault=True,
         ),
         nwarp=dict(argstr='-nwarp %s', ),
-        nwarp_fixdep=dict(argstr='-nwarp_fixdep%s', ),
-        nwarp_fixmot=dict(argstr='-nwarp_fixmot%s', ),
+        nwarp_fixdep=dict(argstr='-nwarp_fixdep%s...', ),
+        nwarp_fixmot=dict(argstr='-nwarp_fixmot%s...', ),
         one_pass=dict(argstr='-onepass', ),
         out_file=dict(
             argstr='-prefix %s',
-            genfile=True,
+            hash_files=False,
+            name_source='in_file',
+            name_template='%s_allineate',
             xor=['allcostx'],
         ),
         out_matrix=dict(