From ee6431fa8aee6e737eaabbaac6b5fb8780a2dcab Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 2 Aug 2018 12:35:06 -0400 Subject: [PATCH 1/5] FIX: Identify collapsing traits and wrap in list --- nipype/pipeline/engine/utils.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/nipype/pipeline/engine/utils.py b/nipype/pipeline/engine/utils.py index 4ec36afe68..887bd9787b 100644 --- a/nipype/pipeline/engine/utils.py +++ b/nipype/pipeline/engine/utils.py @@ -233,12 +233,25 @@ def write_report(node, report_type=None, is_mapnode=False): return +def _protect_collapses(hastraits): + raw = hastraits.trait_get() + cloned = hastraits.clone_traits().trait_get() + + for key in raw: + val = raw[key] + c = cloned[key] + if c != val and hasattr(val, '__getitem__') and c == val[0]: + raw[key] = [val] + + return raw + + def save_resultfile(result, cwd, name): """Save a result pklz file to ``cwd``""" resultsfile = os.path.join(cwd, 'result_%s.pklz' % name) if result.outputs: try: - outputs = result.outputs.trait_get() + outputs = _protect_collapses(result.outputs) except AttributeError: outputs = result.outputs.dictcopy() # outputs was a bunch result.outputs.set(**modify_paths(outputs, relative=True, basedir=cwd)) From 0696280278fed120fc802a1dc9ead83da46b4b14 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Thu, 2 Aug 2018 13:00:50 -0400 Subject: [PATCH 2/5] FIX: Iterate over cloned traits Cloned traits are a subset of the original. --- nipype/pipeline/engine/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/pipeline/engine/utils.py b/nipype/pipeline/engine/utils.py index 887bd9787b..173f38f8d6 100644 --- a/nipype/pipeline/engine/utils.py +++ b/nipype/pipeline/engine/utils.py @@ -237,7 +237,7 @@ def _protect_collapses(hastraits): raw = hastraits.trait_get() cloned = hastraits.clone_traits().trait_get() - for key in raw: + for key in cloned: val = raw[key] c = cloned[key] if c != val and hasattr(val, '__getitem__') and c == val[0]: From 268e47d33bbfdcb1327e2dda0ae1d72dedb5958a Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 2 Aug 2018 14:39:09 -0400 Subject: [PATCH 3/5] RF: Split identification and uncollapsing traits FIX: Improve collapse detection to correctly handle numpy arrays FIX: Uncollapse before modifying paths when loading results --- nipype/pipeline/engine/utils.py | 35 ++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/nipype/pipeline/engine/utils.py b/nipype/pipeline/engine/utils.py index 173f38f8d6..a2fe280a96 100644 --- a/nipype/pipeline/engine/utils.py +++ b/nipype/pipeline/engine/utils.py @@ -233,28 +233,45 @@ def write_report(node, report_type=None, is_mapnode=False): return -def _protect_collapses(hastraits): +def _identify_collapses(hastraits): raw = hastraits.trait_get() cloned = hastraits.clone_traits().trait_get() + collapsed = set() for key in cloned: - val = raw[key] - c = cloned[key] - if c != val and hasattr(val, '__getitem__') and c == val[0]: - raw[key] = [val] + orig = raw[key] + new = cloned[key] + if isinstance(orig, list) and len(orig) == 1 and ( + not np.array_equal(orig, new) and np.array_equal(orig[0], new)): + collapsed.add(key) + + return collapsed + - return raw +def _uncollapse(indexable, collapsed): + for key in indexable: + if key in collapsed: + indexable[key] = [indexable[key]] + return indexable + + +def _protect_collapses(hastraits): + collapsed = _identify_collapses(hastraits) + return _uncollapse(hastraits.trait_get(), collapsed) def save_resultfile(result, cwd, name): """Save a result pklz file to ``cwd``""" resultsfile = os.path.join(cwd, 'result_%s.pklz' % name) if result.outputs: + collapsed = set() try: - outputs = _protect_collapses(result.outputs) + collapsed = _identify_collapses(result.outputs) + outputs = _uncollapse(result.outputs.trait_get(), collapsed) except AttributeError: outputs = result.outputs.dictcopy() # outputs was a bunch - result.outputs.set(**modify_paths(outputs, relative=True, basedir=cwd)) + outputs = modify_paths(outputs, relative=True, basedir=cwd) + result.outputs.set(**_uncollapse(outputs, collapsed)) savepkl(resultsfile, result) logger.debug('saved results in %s', resultsfile) @@ -306,7 +323,7 @@ def load_resultfile(path, name): else: if result.outputs: try: - outputs = result.outputs.trait_get() + outputs = _protect_collapses(result.outputs) except AttributeError: outputs = result.outputs.dictcopy() # outputs == Bunch try: From 2e5e3bc12125dd98baaf6038c6853498dbe47f3b Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 2 Aug 2018 15:10:21 -0400 Subject: [PATCH 4/5] DOC: Describe double collapse issue --- nipype/pipeline/engine/utils.py | 41 +++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/nipype/pipeline/engine/utils.py b/nipype/pipeline/engine/utils.py index a2fe280a96..cc47de5d42 100644 --- a/nipype/pipeline/engine/utils.py +++ b/nipype/pipeline/engine/utils.py @@ -234,6 +234,18 @@ def write_report(node, report_type=None, is_mapnode=False): def _identify_collapses(hastraits): + """ Identify traits that will collapse when being set to themselves. + + ``OutputMultiObject``s automatically unwrap a list of length 1 to directly + reference the element of that list. + If that element is itself a list of length 1, then the following will + result in modified values. + + hastraits.trait_set(**hastraits.trait_get()) + + Cloning performs this operation on a copy of the original traited object, + allowing us to identify traits that will be affected. + """ raw = hastraits.trait_get() cloned = hastraits.clone_traits().trait_get() @@ -241,6 +253,8 @@ def _identify_collapses(hastraits): for key in cloned: orig = raw[key] new = cloned[key] + # Allow numpy to handle the equality checks, as mixed lists and arrays + # can be problematic. if isinstance(orig, list) and len(orig) == 1 and ( not np.array_equal(orig, new) and np.array_equal(orig[0], new)): collapsed.add(key) @@ -249,6 +263,17 @@ def _identify_collapses(hastraits): def _uncollapse(indexable, collapsed): + """ Wrap collapsible values in a list to prevent double-collapsing. + + Should be used with _identify_collapses to provide the following + idempotent operation: + + collapsed = _identify_collapses(hastraits) + hastraits.trait_set(**_uncollapse(hastraits.trait_get(), collapsed)) + + NOTE: Modifies object in-place, in addition to returning it. + """ + for key in indexable: if key in collapsed: indexable[key] = [indexable[key]] @@ -256,6 +281,12 @@ def _uncollapse(indexable, collapsed): def _protect_collapses(hastraits): + """ A collapse-protected replacement for hastraits.trait_get() + + May be used as follows to provide an idempotent trait_set: + + hastraits.trait_set(**_protect_collapses(hastraits)) + """ collapsed = _identify_collapses(hastraits) return _uncollapse(hastraits.trait_get(), collapsed) @@ -264,14 +295,16 @@ def save_resultfile(result, cwd, name): """Save a result pklz file to ``cwd``""" resultsfile = os.path.join(cwd, 'result_%s.pklz' % name) if result.outputs: - collapsed = set() try: collapsed = _identify_collapses(result.outputs) outputs = _uncollapse(result.outputs.trait_get(), collapsed) + # Double-protect tosave so that the original, uncollapsed trait + # is saved in the pickle file. Thus, when the loading process + # collapses, the original correct value is loaded. + tosave = _uncollapse(outputs.copy(), collapsed) except AttributeError: - outputs = result.outputs.dictcopy() # outputs was a bunch - outputs = modify_paths(outputs, relative=True, basedir=cwd) - result.outputs.set(**_uncollapse(outputs, collapsed)) + tosave = outputs = result.outputs.dictcopy() # outputs was a bunch + result.outputs.set(**modify_paths(tosave, relative=True, basedir=cwd)) savepkl(resultsfile, result) logger.debug('saved results in %s', resultsfile) From c50839ee31494714cd93f2715bc3220131bc3215 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 2 Aug 2018 15:10:35 -0400 Subject: [PATCH 5/5] TEST: Test double collapse --- nipype/pipeline/engine/tests/test_nodes.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/nipype/pipeline/engine/tests/test_nodes.py b/nipype/pipeline/engine/tests/test_nodes.py index 4a04b94766..ea03fe69ae 100644 --- a/nipype/pipeline/engine/tests/test_nodes.py +++ b/nipype/pipeline/engine/tests/test_nodes.py @@ -290,3 +290,18 @@ def test_inputs_removal(tmpdir): n1.overwrite = True n1.run() assert not tmpdir.join(n1.name, 'file1.txt').check() + + +def test_outputmultipath_collapse(tmpdir): + """Test an OutputMultiPath whose initial value is ``[[x]]`` to ensure that + it is returned as ``[x]``, regardless of how accessed.""" + select_if = niu.Select(inlist=[[1, 2, 3], [4]], index=1) + select_nd = pe.Node(niu.Select(inlist=[[1, 2, 3], [4]], index=1), + name='select_nd') + + ifres = select_if.run() + ndres = select_nd.run() + + assert ifres.outputs.out == [4] + assert ndres.outputs.out == [4] + assert select_nd.result.outputs.out == [4]