New behaviour of Merge interface with breaks generic cases where numinputs=1 at runtime

[tclose]

Summary

In the current HEAD, the new behaviour of the Merge interface when numinputs==1 breaks cases (such as mine) where the numinputs is set at runtime depending on parameters of the workflow creation code. A better way to handle this would be to see if in_lists is defined or perhaps use numinputs==None.

Actual behavior

TypeError: '_Undefined' object is not iterable

Expected behavior

in1 is taken to be the only element of the merged list

How to replicate the behavior

import nipype.pipeline.engine as pe
from nipype.interfaces.utility.base import Merge

merge  = pe.Node(Merge(numinputs=1), name='merge')
merge.inputs.in1 = 1

workflow = pe.Workflow(name='workflow')
workflow.add_nodes([merge])
workflow.run()

Platform details:

please paste the output of: python -c "import nipype; print(nipype.get_info()); print(nipype.__version__)"

{'nibabel_version': '2.0.1', 'sys_executable': '/usr/local/opt/python/bin/python2.7', 'networkx_version': '1.11', 'numpy_version': '1.12.0', 'sys_platform': 'darwin', 'sys_version': '2.7.13 (default, Dec 17 2016, 23:03:43) \n[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]', 'commit_source': u'repository', 'commit_hash': '368e184', 'pkg_path': '/Users/tclose/git/nipype/nipype', 'nipype_version': u'0.13.0-g368e184.dev', 'traits_version': '4.6.0', 'scipy_version': '0.14.0'}
0.13.0-g368e184.dev

[mgxd]

@tclose this seems more like a design idea - the fact that if a sole list is passed, calling it in1 would imply other potential in connections. When Merge is used for this purpose, having a separate keyword (in_lists) makes it more explicit. However, we should think of adding more warnings/errors when the interface isn't properly defined

[effigies]

Yes. My original PR was designed to make Merge(0) meaningful without affecting existing pipelines, but after discussion it was decided that because Merge(1) is equivalent to IdentityInterface (but changing the field name), it wasn't serving a useful function.

However, given that somebody actually is using that behavior, perhaps it's worth reconsidering Merge(0)?

@tclose What's your use case? Regardless of whether the old behavior is restored, we may be able to come up with a sensible alternative using the new interface that isn't (much) more complicated than your existing one. For instance, supposing your maximum is numinputs=3, perhaps you could do:

merge = pe.Node(Merge(3), name='merge')

if needed_inputs < 3:
    merge.inputs.in3 = []
if needed_inputs < 2:
    merge.inputs.in2 = []

Or something of the sort.

[tclose]

My use case is a little complex but I will try to convey it in a few lines. I am writing a package to integrate nipype with our archive (XNAT), which implicitly generates workflows to produce derived datasets that are not present for a given subject. A book-keeping node at the end of the generated workflow is used to merge the subject outputs into a single list, but when there is only one subject to process this list only contains a single input (as in my example).

In general, it seems a bad idea to break backwards compatibility (even if mine is an unusual use-case) for what you are trying to achieve here. I would also find it cleaner and more intuitive if numinputs=None were used to specify that the in_lists is to be used instead of 'in1', 'in2', etc.. (numinputs==0 could produce an empty list). Also, perhaps a completely separate MergeLists interface would be a better idea as the behaviour is significantly different?

[satra]

@tclose - thanks for the feedback. i apologize for the decision to break backwards compatibility here without clarification.

moving forward, we can restore Merge, with 0 producing an empty list.

we can also introduce a few flags to deal with flattening: flatten_outputs, flatten_inputs

flatten_inputs will flatten before merge, while flatten_outputs will flatten after merge

[satra]

by default both will be set to undefined to maintain backwards compatibility.

[tclose]

@satra Thanks, that would be great.