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,13 +25,13 @@
 from ... import logging, config, LooseVersion
 from ...utils.filemanip import (
     Path,
+    indirectory,
     relpath,
     makedirs,
     fname_presuffix,
     to_str,
     ensure_list,
     get_related_files,
-    FileNotFoundError,
     save_json,
     savepkl,
     loadpkl,
@@ -41,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
@@ -227,52 +228,82 @@ def write_report(node, report_type=None, is_mapnode=False):
     return
 
 
-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)
-    savepkl(resultsfile, result)
-    logger.debug('saved results in %s', resultsfile)
+    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
 
-def load_resultfile(results_file):
+    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(results_file, 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
     results_file = Path(results_file)
+    aggregate = True
     result = None
     attribute_error = False
-    if results_file.exists():
+
+    if not results_file.exists():
+        return result, aggregate, attribute_error
+
+    with indirectory(str(results_file.parent)):
         try:
             result = loadpkl(results_file)
-        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')
+        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:
             aggregate = False
 
-    logger.debug('Aggregate: %s', aggregate)
+    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,
+                                        results_file.parent)
+            setattr(result.outputs, trait_name, value)
+
     return result, aggregate, attribute_error