Merge pull request #2384 from oesteban/fix/2380

FIX: Change to interface workdir within Interface.run() instead Node

Author: satra

nipype/interfaces/base/core.py

@@ -33,7 +33,7 @@
 
 from ... import config, logging, LooseVersion
 from ...utils.provenance import write_provenance
-from ...utils.misc import trim, str2bool
+from ...utils.misc import trim, str2bool, rgetcwd
 from ...utils.filemanip import (FileNotFoundError, split_filename, read_stream,
                                 which, get_dependencies, canonicalize_env as
                                 _canonicalize_env)
@@ -438,14 +438,16 @@ def _duecredit_cite(self):
             r['path'] = self.__module__
             due.cite(**r)
 
-    def run(self, **inputs):
+    def run(self, cwd=None, **inputs):
         """Execute this interface.
 
         This interface will not raise an exception if runtime.returncode is
         non-zero.
 
         Parameters
         ----------
+
+        cwd : specify a folder where the interface should be run
         inputs : allows the interface settings to be updated
 
         Returns
@@ -455,6 +457,13 @@ def run(self, **inputs):
         """
         from ...utils.profiler import ResourceMonitor
 
+        # Tear-up: get current and prev directories
+        syscwd = rgetcwd(error=False)  # Recover when wd does not exist
+        if cwd is None:
+            cwd = syscwd
+
+        os.chdir(cwd)  # Change to the interface wd
+
         enable_rm = config.resource_monitor and self.resource_monitor
         force_raise = not getattr(self.inputs, 'ignore_exception', False)
         self.inputs.trait_set(**inputs)
@@ -471,7 +480,8 @@ def run(self, **inputs):
             env['DISPLAY'] = config.get_display()
 
         runtime = Bunch(
-            cwd=os.getcwd(),
+            cwd=cwd,
+            prevcwd=syscwd,
             returncode=None,
             duration=None,
             environ=env,
@@ -556,6 +566,7 @@ def run(self, **inputs):
                         'rss_GiB': (vals[:, 2] / 1024).tolist(),
                         'vms_GiB': (vals[:, 3] / 1024).tolist(),
                     }
+            os.chdir(syscwd)
 
         return results
 

nipype/pipeline/engine/nodes.py

@@ -449,26 +449,13 @@ def run(self, updatehash=False):
         savepkl(op.join(outdir, '_node.pklz'), self)
         savepkl(op.join(outdir, '_inputs.pklz'), self.inputs.get_traitsfree())
 
-        try:
-            cwd = os.getcwd()
-        except OSError:
-            # Changing back to cwd is probably not necessary
-            # but this makes sure there's somewhere to change to.
-            cwd = op.split(outdir)[0]
-            logger.warning(
-                'Current folder "%s" does not exist, changing to "%s" instead.',
-                os.getenv('PWD', 'unknown'), cwd)
-
-        os.chdir(outdir)
         try:
             result = self._run_interface(execute=True)
         except Exception:
             logger.warning('[Node] Error on "%s" (%s)', self.fullname, outdir)
             # Tear-up after error
             os.remove(hashfile_unfinished)
             raise
-        finally:  # Ensure we come back to the original CWD
-            os.chdir(cwd)
 
         # Tear-up after success
         shutil.move(hashfile_unfinished, hashfile)
@@ -586,17 +573,18 @@ def _run_command(self, execute, copyfiles=True):
                 logger.info("[Node] Cached - collecting precomputed outputs")
                 return result
 
+        outdir = self.output_dir()
         # Run command: either execute is true or load_results failed.
-        runtime = Bunch(
-            returncode=1,
-            environ=dict(os.environ),
-            hostname=socket.gethostname())
         result = InterfaceResult(
             interface=self._interface.__class__,
-            runtime=runtime,
+            runtime=Bunch(
+                cwd=outdir,
+                returncode=1,
+                environ=dict(os.environ),
+                hostname=socket.gethostname()
+            ),
             inputs=self._interface.inputs.get_traitsfree())
 
-        outdir = self.output_dir()
         if copyfiles:
             self._originputs = deepcopy(self._interface.inputs)
             self._copyfiles_to_wd(execute=execute)
@@ -618,7 +606,7 @@ def _run_command(self, execute, copyfiles=True):
             message += ', a CommandLine Interface with command:\n{}'.format(cmd)
         logger.info(message)
         try:
-            result = self._interface.run()
+            result = self._interface.run(cwd=outdir)
         except Exception as msg:
             result.runtime.stderr = '%s\n\n%s'.format(
                 getattr(result.runtime, 'stderr', ''), msg)

nipype/pipeline/engine/utils.py

@@ -189,6 +189,7 @@ def write_report(node, report_type=None, is_mapnode=False):
         'hostname': result.runtime.hostname,
         'duration': result.runtime.duration,
         'working_dir': result.runtime.cwd,
+        'prev_wd': getattr(result.runtime, 'prevcwd', '<not-set>'),
     }
 
     if hasattr(result.runtime, 'cmdline'):

nipype/utils/misc.py

@@ -7,9 +7,11 @@
                         absolute_import)
 from builtins import next, str
 
+import os
 import sys
 import re
 from collections import Iterator
+from warnings import warn
 
 from distutils.version import LooseVersion
 
@@ -301,3 +303,25 @@ def dict_diff(dold, dnew, indent=0):
         diff.insert(diffkeys, "Some dictionary entries had differing values:")
 
     return textwrap_indent('\n'.join(diff), ' ' * indent)
+
+
+def rgetcwd(error=True):
+    """
+    Robust replacement for getcwd when folders get removed
+    If error==True, this is just an alias for os.getcwd()
+    """
+    if error:
+        return os.getcwd()
+
+    try:
+        cwd = os.getcwd()
+    except OSError as exc:
+        # Changing back to cwd is probably not necessary
+        # but this makes sure there's somewhere to change to.
+        cwd = os.getenv('PWD')
+        if cwd is None:
+            raise OSError((
+                exc.errno, 'Current directory does not exist anymore, '
+                'and nipype was not able to guess it from the environment'))
+        warn('Current folder does not exist, replacing with "%s" instead.' % cwd)
+    return cwd

nipype/utils/tests/test_misc.py

@@ -4,6 +4,8 @@
 from future import standard_library
 standard_library.install_aliases()
 
+import os
+from shutil import rmtree
 from builtins import next
 
 import pytest
@@ -60,3 +62,30 @@ def test_flatten():
 
     back = unflatten([], [])
     assert back == []
+
+
+def test_rgetcwd(monkeypatch, tmpdir):
+    from ..misc import rgetcwd
+    oldpath = tmpdir.strpath
+    tmpdir.mkdir("sub").chdir()
+    newpath = os.getcwd()
+
+    # Path still there
+    assert rgetcwd() == newpath
+
+    # Remove path
+    rmtree(newpath, ignore_errors=True)
+    with pytest.raises(OSError):
+        os.getcwd()
+
+    monkeypatch.setenv('PWD', oldpath)
+    assert rgetcwd(error=False) == oldpath
+
+    # Test when error should be raised
+    with pytest.raises(OSError):
+        rgetcwd()
+
+    # Deleted env variable
+    monkeypatch.delenv('PWD')
+    with pytest.raises(OSError):
+        rgetcwd(error=False)

nipype/utils/tests/test_provenance.py

@@ -11,9 +11,10 @@
 from nipype.utils.provenance import ProvStore, safe_encode
 
 
-def test_provenance():
-    ps = ProvStore()
+def test_provenance(tmpdir):
     from nipype.interfaces.base import CommandLine
+    tmpdir.chdir()
+    ps = ProvStore()
     results = CommandLine('echo hello').run()
     ps.add_results(results)
     provn = ps.g.get_provn()