enh: add resolving to the results loader and rebasing to saver

Fixes #2944.

Author: oesteban

nipype/pipeline/engine/tests/test_utils.py

@@ -224,3 +224,62 @@ def test_mapnode_crash3(tmpdir):
     wf.config["execution"]["crashdump_dir"] = os.getcwd()
     with pytest.raises(RuntimeError):
         wf.run(plugin='Linear')
+
+class StrPathConfuserInputSpec(nib.TraitedSpec):
+    in_str = nib.traits.String()
+
+
+class StrPathConfuserOutputSpec(nib.TraitedSpec):
+    out_tuple = nib.traits.Tuple(nib.File, nib.traits.String)
+    out_dict_path = nib.traits.Dict(nib.traits.String, nib.File(exists=True))
+    out_dict_str = nib.traits.DictStrStr()
+    out_list = nib.traits.List(nib.traits.String)
+    out_str = nib.traits.String()
+    out_path = nib.File(exists=True)
+
+
+class StrPathConfuser(nib.SimpleInterface):
+    input_spec = StrPathConfuserInputSpec
+    output_spec = StrPathConfuserOutputSpec
+
+    def _run_interface(self, runtime):
+        out_path = os.path.abspath(os.path.basename(self.inputs.in_str) + '_path')
+        open(out_path, 'w').close()
+        self._results['out_str'] = self.inputs.in_str
+        self._results['out_path'] = out_path
+        self._results['out_tuple'] = (out_path, self.inputs.in_str)
+        self._results['out_dict_path'] = {self.inputs.in_str: out_path}
+        self._results['out_dict_str'] = {self.inputs.in_str: self.inputs.in_str}
+        self._results['out_list'] = [self.inputs.in_str] * 2
+        return runtime
+
+
+def test_modify_paths_bug(tmpdir):
+    """
+    There was a bug in which, if the current working directory contained a file with the name
+    of an output String, the string would get transformed into a path, and generally wreak havoc.
+    This attempts to replicate that condition, using an object with strings and paths in various
+    trait configurations, to ensure that the guards added resolve the issue.
+    Please see https://github.com/nipy/nipype/issues/2944 for more details.
+    """
+    tmpdir.chdir()
+
+    spc = pe.Node(StrPathConfuser(in_str='2'), name='spc')
+
+    open('2', 'w').close()
+
+    outputs = spc.run().outputs
+
+    # Basic check that string was not manipulated
+    out_str = outputs.out_str
+    assert out_str == '2'
+
+    # Check path exists and is absolute
+    out_path = outputs.out_path
+    assert os.path.isabs(out_path)
+
+    # Assert data structures pass through correctly
+    assert outputs.out_tuple == (out_path, out_str)
+    assert outputs.out_dict_path == {out_str: out_path}
+    assert outputs.out_dict_str == {out_str: out_str}
+    assert outputs.out_list == [out_str] * 2

nipype/pipeline/engine/utils.py

@@ -25,6 +25,7 @@
 
 from ... import logging, config, LooseVersion
 from ...utils.filemanip import (
+    indirectory,
     relpath,
     makedirs,
     fname_presuffix,
@@ -40,6 +41,7 @@
 )
 from ...utils.misc import str2bool
 from ...utils.functions import create_function_from_source
+from ...interfaces.base.traits_extension import rebase_path_traits, resolve_path_traits
 from ...interfaces.base import (Bunch, CommandLine, isdefined, Undefined,
                                 InterfaceResult, traits)
 from ...interfaces.utility import IdentityInterface
@@ -284,86 +286,88 @@ def _protect_collapses(hastraits):
     return _uncollapse(hastraits.trait_get(), collapsed)
 
 
-def save_resultfile(result, cwd, name):
-    """Save a result pklz file to ``cwd``"""
+def save_resultfile(result, cwd, name, rebase=True):
+    """Save a result pklz file to ``cwd``."""
+    cwd = os.path.abspath(cwd)
     resultsfile = os.path.join(cwd, 'result_%s.pklz' % name)
-    if result.outputs:
-        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:
-            tosave = outputs = result.outputs.dictcopy()  # outputs was a bunch
-        for k, v in list(modify_paths(tosave, relative=True, basedir=cwd).items()):
-            setattr(result.outputs, k, v)
-
-    savepkl(resultsfile, result)
-    logger.debug('saved results in %s', resultsfile)
-
-    if result.outputs:
-        for k, v in list(outputs.items()):
-            setattr(result.outputs, k, v)
-
-
-def load_resultfile(path, name):
+    logger.debug("Saving results file: '%s'", resultsfile)
+
+    if result.outputs is None:
+        logger.warn('Storing result file without outputs')
+        savepkl(resultsfile, result)
+        return
+
+    try:
+        outputs = result.outputs.trait_get()
+    except AttributeError:
+        logger.debug('Storing non-traited results, skipping rebase of paths')
+        savepkl(resultsfile, result)
+        return
+
+    try:
+        with indirectory(cwd):
+            # All the magic to fix #2944 resides here:
+            for key, val in list(outputs.items()):
+                val = rebase_path_traits(result.outputs.trait(key), val, cwd)
+                setattr(result.outputs, key, val)
+        savepkl(resultsfile, result)
+    finally:
+        # Reset resolved paths from the outputs dict no matter what
+        for key, val in list(outputs.items()):
+            setattr(result.outputs, key, val)
+
+
+def load_resultfile(path, name, resolve=True):
     """
-    Load InterfaceResult file from path
+    Load InterfaceResult file from path.
 
     Parameter
     ---------
-
     path : base_dir of node
     name : name of node
 
     Returns
     -------
-
     result : InterfaceResult structure
     aggregate : boolean indicating whether node should aggregate_outputs
     attribute error : boolean indicating whether there was some mismatch in
         versions of traits used to store result and hence node needs to
         rerun
+
     """
     aggregate = True
-    resultsoutputfile = os.path.join(path, 'result_%s.pklz' % name)
     result = None
     attribute_error = False
-    if os.path.exists(resultsoutputfile):
-        pkl_file = gzip.open(resultsoutputfile, 'rb')
+    resultsoutputfile = os.path.join(path, 'result_%s.pklz' % name)
+    if not os.path.exists(resultsoutputfile):
+        return result, aggregate, attribute_error
+
+    with indirectory(path):
+        pkl_file = gzip.open('result_%s.pklz' % name, 'rb')
         try:
             result = pickle.load(pkl_file)
         except UnicodeDecodeError:
             # Was this pickle created with Python 2.x?
-            pickle.load(pkl_file, fix_imports=True, encoding='utf-8')
-            logger.warning('Successfully loaded pkl in compatibility mode')
-        except (traits.TraitError, AttributeError, ImportError,
-                EOFError) as err:
-            if isinstance(err, (AttributeError, ImportError)):
-                attribute_error = True
-                logger.debug('attribute error: %s probably using '
-                             'different trait pickled file', str(err))
-            else:
-                logger.debug(
-                    'some file does not exist. hence trait cannot be set')
+            result = pickle.load(pkl_file, fix_imports=True, encoding='utf-8')
+            logger.warning('Successfully loaded pkl in compatibility mode.')
+        except (traits.TraitError, EOFError):
+            logger.debug(
+                'some file does not exist. hence trait cannot be set')
+        except (AttributeError, ImportError) as err:
+            attribute_error = True
+            logger.debug('attribute error: %s probably using '
+                         'different trait pickled file', str(err))
         else:
-            if result.outputs:
-                try:
-                    outputs = _protect_collapses(result.outputs)
-                except AttributeError:
-                    outputs = result.outputs.dictcopy()  # outputs == Bunch
-                try:
-                    for k, v in list(modify_paths(outputs, relative=False,
-                                                  basedir=path).items()):
-                        setattr(result.outputs, k, v)
-                except FileNotFoundError:
-                    logger.debug('conversion to full path results in '
-                                 'non existent file')
             aggregate = False
-        pkl_file.close()
-    logger.debug('Aggregate: %s', aggregate)
+        finally:
+            pkl_file.close()
+
+    if resolve and not aggregate:
+        logger.debug('Resolving paths in outputs loaded from results file.')
+        for trait_name, old_value in list(result.outputs.get().items()):
+            value = resolve_path_traits(result.outputs.trait(trait_name), old_value, path)
+            setattr(result.outputs, trait_name, value)
+
     return result, aggregate, attribute_error