From d7f9186ff7cf4fbff60b6df139f26aeccd7839c2 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Tue, 7 Oct 2014 14:57:20 +0200 Subject: [PATCH 1/7] [engine] Allow chained name_source --- nipype/interfaces/base.py | 47 ++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/nipype/interfaces/base.py b/nipype/interfaces/base.py index ef6f8fd1a1..49b1f6ed3f 100644 --- a/nipype/interfaces/base.py +++ b/nipype/interfaces/base.py @@ -24,7 +24,6 @@ from datetime import datetime as dt from dateutil.parser import parse as parseutc from warnings import warn -from nipype.external import six from .traits_extension import (traits, Undefined, TraitDictObject, @@ -542,7 +541,7 @@ def _get_sorteddict(self, object, dictwithhash=False, hash_method=None, out = tuple(out) else: if isdefined(object): - if (hash_files and isinstance(object, six.string_types) and + if (hash_files and isinstance(object, str) and os.path.isfile(object)): if hash_method is None: hash_method = config.get('execution', 'hash_method') @@ -985,7 +984,7 @@ def run(self, **inputs): else: inputs_str = '' - if len(e.args) == 1 and isinstance(e.args[0], six.string_types): + if len(e.args) == 1 and isinstance(e.args[0], str): e.args = (e.args[0] + " ".join([message, inputs_str]),) else: e.args += (message, ) @@ -1469,6 +1468,7 @@ def _format_arg(self, name, trait_spec, value): def _filename_from_source(self, name): trait_spec = self.inputs.trait(name) retval = getattr(self.inputs, name) + if not isdefined(retval) or "%s" in retval: if not trait_spec.name_source: return retval @@ -1478,26 +1478,37 @@ def _filename_from_source(self, name): name_template = trait_spec.name_template if not name_template: name_template = "%s_generated" - if isinstance(trait_spec.name_source, list): - for ns in trait_spec.name_source: - if isdefined(getattr(self.inputs, ns)): - name_source = ns - break + + ns = trait_spec.name_source + while isinstance(ns, list): + if len(ns) > 1: + iflogger.warn('Only one name_source per trait is allowed') + ns = ns[0] + + if not isinstance(ns, basestring): + raise ValueError(('name_source of \'%s\' trait sould be an ' + 'input trait name') % name) + + if isdefined(getattr(self.inputs, ns)): + name_source = ns + source = getattr(self.inputs, name_source) + while isinstance(source, list): + source = source[0] + + # special treatment for files + try: + _, base, _ = split_filename(source) + except AttributeError: + base = source else: - name_source = trait_spec.name_source - source = getattr(self.inputs, name_source) - while isinstance(source, list): - source = source[0] - #special treatment for files - try: - _, base, _ = split_filename(source) - except AttributeError: - base = source + base = self._filename_from_source(ns) + retval = name_template % base _, _, ext = split_filename(retval) if trait_spec.keep_extension and ext: return retval return self._overload_extension(retval, name) + return retval def _gen_filename(self, name): @@ -1513,7 +1524,7 @@ def _list_outputs(self): outputs = self.output_spec().get() for name, trait_spec in traits.iteritems(): out_name = name - if trait_spec.output_name != None: + if trait_spec.output_name is not None: out_name = trait_spec.output_name outputs[out_name] = \ os.path.abspath(self._filename_from_source(name)) From 2b1d259a3f5d79f8bd37bf8fe89c15df6e14ecc5 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Tue, 7 Oct 2014 15:02:32 +0200 Subject: [PATCH 2/7] revert outdated changes to master --- nipype/interfaces/base.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/base.py b/nipype/interfaces/base.py index 49b1f6ed3f..06f275224a 100644 --- a/nipype/interfaces/base.py +++ b/nipype/interfaces/base.py @@ -24,6 +24,7 @@ from datetime import datetime as dt from dateutil.parser import parse as parseutc from warnings import warn +from nipype.external import six from .traits_extension import (traits, Undefined, TraitDictObject, @@ -541,7 +542,7 @@ def _get_sorteddict(self, object, dictwithhash=False, hash_method=None, out = tuple(out) else: if isdefined(object): - if (hash_files and isinstance(object, str) and + if (hash_files and isinstance(object, six.string_types) and os.path.isfile(object)): if hash_method is None: hash_method = config.get('execution', 'hash_method') @@ -984,7 +985,7 @@ def run(self, **inputs): else: inputs_str = '' - if len(e.args) == 1 and isinstance(e.args[0], str): + if len(e.args) == 1 and isinstance(e.args[0], six.string_types): e.args = (e.args[0] + " ".join([message, inputs_str]),) else: e.args += (message, ) From 06c2298a69280a31c249873cf76a45ed5b686aa3 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 9 Oct 2014 19:09:15 +0200 Subject: [PATCH 3/7] use six.string_types --- nipype/interfaces/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/base.py b/nipype/interfaces/base.py index 06f275224a..26132d452c 100644 --- a/nipype/interfaces/base.py +++ b/nipype/interfaces/base.py @@ -1486,7 +1486,7 @@ def _filename_from_source(self, name): iflogger.warn('Only one name_source per trait is allowed') ns = ns[0] - if not isinstance(ns, basestring): + if not isinstance(ns, six.string_types): raise ValueError(('name_source of \'%s\' trait sould be an ' 'input trait name') % name) From 5b6066c9babbdca20bf102952bd907feac6676be Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Wed, 11 Feb 2015 20:58:56 +0100 Subject: [PATCH 4/7] add regression test for chained namesource --- nipype/interfaces/tests/test_base.py | 31 +++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/nipype/interfaces/tests/test_base.py b/nipype/interfaces/tests/test_base.py index a509137309..5e84d75af8 100644 --- a/nipype/interfaces/tests/test_base.py +++ b/nipype/interfaces/tests/test_base.py @@ -172,11 +172,13 @@ class DeprecationSpec3(nib.TraitedSpec): yield assert_equal, spec_instance.foo, Undefined yield assert_equal, spec_instance.bar, 1 + def test_namesource(): tmp_infile = setup_file() tmpd, nme, ext = split_filename(tmp_infile) pwd = os.getcwd() os.chdir(tmpd) + class spec2(nib.CommandLineInputSpec): moo = nib.File(name_source=['doo'], hash_files=False, argstr="%s", position=2) @@ -196,6 +198,33 @@ class TestName(nib.CommandLine): os.chdir(pwd) teardown_file(tmpd) + +def test_chained_namesource(): + tmp_infile = setup_file() + tmpd, nme, ext = split_filename(tmp_infile) + pwd = os.getcwd() + os.chdir(tmpd) + + class spec2(nib.CommandLineInputSpec): + doo = nib.File(exists=True, argstr="%s", position=1) + moo = nib.File(name_source=['doo'], hash_files=False, argstr="%s", + position=2, name_template='%s_mootpl') + poo = nib.File(name_source=['moo'], hash_files=False, + argstr="%s", position=3) + + class TestName(nib.CommandLine): + _cmd = "mycommand" + input_spec = spec2 + + testobj = TestName() + testobj.inputs.doo = tmp_infile + yield assert_true, '%s_mootpl' % nme in testobj.cmdline + yield assert_true, '%s_mootpl_generated' % nme in testobj.cmdline + + os.chdir(pwd) + teardown_file(tmpd) + + def checknose(): """check version of nose for known incompatability""" mod = __import__('nose') @@ -536,4 +565,4 @@ def test_global_CommandLine_output(): res = ci.run() yield assert_equal, res.runtime.stdout, '' os.chdir(pwd) - teardown_file(tmpd) \ No newline at end of file + teardown_file(tmpd) From ebb46cdde0558167b56dfbb1d855447c5885b37d Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Wed, 11 Feb 2015 21:00:47 +0100 Subject: [PATCH 5/7] update CHANGES --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index c8163e0440..b6005496f0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,7 @@ Next release ============ +* ENH: Inputs with name_source can be now chained in cascade (https://github.com/nipy/nipype/pull/938) * FIX: Amend create_tbss_non_fa() workflow to match FSL's tbss_non_fa command. (https://github.com/nipy/nipype/pull/1033) * FIX: remove unused mandatory flag from spm normalize (https://github.com/nipy/nipype/pull/1048) * ENH: Update ANTSCorticalThickness interface (https://github.com/nipy/nipype/pull/1013) From c9e7b092cbe9dbab1c77fa6babb4fa2c0eb20a40 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 12 Feb 2015 15:47:17 +0100 Subject: [PATCH 6/7] added new tests, updated documentation --- doc/devel/interface_specs.rst | 3 ++ nipype/interfaces/base.py | 18 +++++++++-- nipype/interfaces/tests/test_base.py | 45 ++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/doc/devel/interface_specs.rst b/doc/devel/interface_specs.rst index 12e6d1227d..155a14ae10 100644 --- a/doc/devel/interface_specs.rst +++ b/doc/devel/interface_specs.rst @@ -358,6 +358,9 @@ CommandLine ``name_source`` Indicates the list of input fields from which the value of the current File output variable will be drawn. This input field must be the name of a File. + Chaining is allowed, meaning that an input field can point to another as + ``name_source``, which also points as ``name_source`` to a third field. + In this situation, the templates for substitutions are also accumulated. ``name_template`` By default a ``%s_generated`` template is used to create the output diff --git a/nipype/interfaces/base.py b/nipype/interfaces/base.py index 60383aab45..b2ea92c750 100644 --- a/nipype/interfaces/base.py +++ b/nipype/interfaces/base.py @@ -47,6 +47,12 @@ __docformat__ = 'restructuredtext' +class NipypeInterfaceError(Exception): + def __init__(self, value): + self.value = value + def __str__(self): + return repr(self.value) + def _lock_files(): tmpdir = '/tmp' pattern = '.X*-lock' @@ -1510,7 +1516,10 @@ def _format_arg(self, name, trait_spec, value): # Append options using format string. return argstr % value - def _filename_from_source(self, name): + def _filename_from_source(self, name, chain=None): + if chain is None: + chain = [] + trait_spec = self.inputs.trait(name) retval = getattr(self.inputs, name) @@ -1546,8 +1555,13 @@ def _filename_from_source(self, name): except AttributeError: base = source else: - base = self._filename_from_source(ns) + if name in chain: + raise NipypeInterfaceError('Mutually pointing name_sources') + + chain.append(name) + base = self._filename_from_source(ns, chain) + chain = None retval = name_template % base _, _, ext = split_filename(retval) if trait_spec.keep_extension and ext: diff --git a/nipype/interfaces/tests/test_base.py b/nipype/interfaces/tests/test_base.py index 5e84d75af8..525391704a 100644 --- a/nipype/interfaces/tests/test_base.py +++ b/nipype/interfaces/tests/test_base.py @@ -218,8 +218,49 @@ class TestName(nib.CommandLine): testobj = TestName() testobj.inputs.doo = tmp_infile - yield assert_true, '%s_mootpl' % nme in testobj.cmdline - yield assert_true, '%s_mootpl_generated' % nme in testobj.cmdline + res = testobj.cmdline + yield assert_true, '%s' % tmp_infile in res + yield assert_true, '%s_mootpl ' % nme in res + yield assert_true, '%s_mootpl_generated' % nme in res + + os.chdir(pwd) + teardown_file(tmpd) + + +def test_cycle_namesource(): + tmp_infile = setup_file() + tmpd, nme, ext = split_filename(tmp_infile) + pwd = os.getcwd() + os.chdir(tmpd) + + from unittest import TestCase + + class spec3(nib.CommandLineInputSpec): + moo = nib.File(name_source=['doo'], hash_files=False, argstr="%s", + position=1, name_template='%s_mootpl') + poo = nib.File(name_source=['moo'], hash_files=False, + argstr="%s", position=2) + doo = nib.File(name_source=['poo'], hash_files=False, + argstr="%s", position=3) + + class TestCycle(nib.CommandLine): + _cmd = "mycommand" + input_spec = spec3 + + def get_cmd(self): + return self.cmdline + + # Check that an exception is raised + to0 = TestCycle() + yield assert_raises, nib.NipypeInterfaceError, to0.get_cmd + + # Check that loop can be broken by setting one of the inputs + to1 = TestCycle() + to1.inputs.poo = tmp_infile + res = to1.cmdline + yield assert_true, '%s' % tmp_infile in res + yield assert_true, '%s_generated ' % nme in res + yield assert_true, '%s_generated_mootpl' % nme in res os.chdir(pwd) teardown_file(tmpd) From 5f991fb7377a37da2db2e55ef9eaef5e6ae50e9e Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Thu, 12 Feb 2015 17:03:41 +0100 Subject: [PATCH 7/7] fixed regression tests --- nipype/interfaces/tests/test_base.py | 48 ++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/nipype/interfaces/tests/test_base.py b/nipype/interfaces/tests/test_base.py index 525391704a..112754ea8e 100644 --- a/nipype/interfaces/tests/test_base.py +++ b/nipype/interfaces/tests/test_base.py @@ -227,14 +227,12 @@ class TestName(nib.CommandLine): teardown_file(tmpd) -def test_cycle_namesource(): +def test_cycle_namesource1(): tmp_infile = setup_file() tmpd, nme, ext = split_filename(tmp_infile) pwd = os.getcwd() os.chdir(tmpd) - from unittest import TestCase - class spec3(nib.CommandLineInputSpec): moo = nib.File(name_source=['doo'], hash_files=False, argstr="%s", position=1, name_template='%s_mootpl') @@ -247,19 +245,51 @@ class TestCycle(nib.CommandLine): _cmd = "mycommand" input_spec = spec3 - def get_cmd(self): - return self.cmdline - # Check that an exception is raised to0 = TestCycle() - yield assert_raises, nib.NipypeInterfaceError, to0.get_cmd + not_raised = True + try: + to0.cmdline + except nib.NipypeInterfaceError: + not_raised = False + yield assert_false, not_raised + + os.chdir(pwd) + teardown_file(tmpd) + +def test_cycle_namesource2(): + tmp_infile = setup_file() + tmpd, nme, ext = split_filename(tmp_infile) + pwd = os.getcwd() + os.chdir(tmpd) + + + class spec3(nib.CommandLineInputSpec): + moo = nib.File(name_source=['doo'], hash_files=False, argstr="%s", + position=1, name_template='%s_mootpl') + poo = nib.File(name_source=['moo'], hash_files=False, + argstr="%s", position=2) + doo = nib.File(name_source=['poo'], hash_files=False, + argstr="%s", position=3) + + class TestCycle(nib.CommandLine): + _cmd = "mycommand" + input_spec = spec3 # Check that loop can be broken by setting one of the inputs to1 = TestCycle() to1.inputs.poo = tmp_infile - res = to1.cmdline + + not_raised = True + try: + res = to1.cmdline + except nib.NipypeInterfaceError: + not_raised = False + print res + + yield assert_true, not_raised yield assert_true, '%s' % tmp_infile in res - yield assert_true, '%s_generated ' % nme in res + yield assert_true, '%s_generated' % nme in res yield assert_true, '%s_generated_mootpl' % nme in res os.chdir(pwd)