From 9ccb07675d1f32abc872398c85921422c8089087 Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jan 2018 11:13:31 -0800 Subject: [PATCH 1/6] new robust getcwd --- nipype/utils/misc.py | 24 ++++++++++++++++++++++++ nipype/utils/tests/test_misc.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/nipype/utils/misc.py b/nipype/utils/misc.py index f7273144df..a7487539d6 100644 --- a/nipype/utils/misc.py +++ b/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 diff --git a/nipype/utils/tests/test_misc.py b/nipype/utils/tests/test_misc.py index ffe1018ac9..8896039763 100644 --- a/nipype/utils/tests/test_misc.py +++ b/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) From 00d5a8144fc0c95ef85d7bcc69f513e34cd4bc61 Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jan 2018 11:41:10 -0800 Subject: [PATCH 2/6] run test in temp dir --- nipype/interfaces/base/core.py | 17 +++++++++++++--- nipype/pipeline/engine/nodes.py | 28 ++++++++------------------- nipype/utils/tests/test_provenance.py | 5 +++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/nipype/interfaces/base/core.py b/nipype/interfaces/base/core.py index 34f650cde2..3aa84b9b3f 100644 --- a/nipype/interfaces/base/core.py +++ b/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,7 +438,7 @@ 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 @@ -446,6 +446,8 @@ def run(self, **inputs): 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 diff --git a/nipype/pipeline/engine/nodes.py b/nipype/pipeline/engine/nodes.py index e013702f8a..054f9a622c 100644 --- a/nipype/pipeline/engine/nodes.py +++ b/nipype/pipeline/engine/nodes.py @@ -449,17 +449,6 @@ 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: @@ -467,8 +456,6 @@ def run(self, updatehash=False): # 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) diff --git a/nipype/utils/tests/test_provenance.py b/nipype/utils/tests/test_provenance.py index 1b435760e1..1d7233907a 100644 --- a/nipype/utils/tests/test_provenance.py +++ b/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() From 653f690f11df92582b08fa1f9969f41f20ec6347 Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jan 2018 11:41:31 -0800 Subject: [PATCH 3/6] update nilearn interface to use runtime.cwd --- nipype/interfaces/nilearn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/nilearn.py b/nipype/interfaces/nilearn.py index 3492e2e233..1d7bdff0e5 100644 --- a/nipype/interfaces/nilearn.py +++ b/nipype/interfaces/nilearn.py @@ -106,7 +106,7 @@ def _run_interface(self, runtime): region_signals.astype(str))) # save output - self._results['out_file'] = os.path.abspath(self.inputs.out_file) + self._results['out_file'] = os.path.join(runtime.cwd, self.inputs.out_file) np.savetxt( self._results['out_file'], output, fmt=b'%s', delimiter='\t') return runtime From 533c247b518cbf1569b927610a333afd74a0fef6 Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jan 2018 11:46:54 -0800 Subject: [PATCH 4/6] update CHANGES (close #2380) --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index fa17166884..e43c421f5f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,7 @@ Upcoming release (0.14.1) ========================= +* FIX: Change to interface workdir within Interface.run() instead Node (https://github.com/nipy/nipype/pull/2384) * FIX: MultiProc starting workers at dubious wd (https://github.com/nipy/nipype/pull/2368) * REF+FIX: Move BIDSDataGrabber to `interfaces.io` + fix correct default behavior (https://github.com/nipy/nipype/pull/2336) * ENH: Add AFNI interface for 3dConvertDset (https://github.com/nipy/nipype/pull/2337) From f4c3db723b405193b1406a7c512780a8615ab32e Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jan 2018 11:55:15 -0800 Subject: [PATCH 5/6] roll back nilearn interface --- nipype/interfaces/nilearn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nipype/interfaces/nilearn.py b/nipype/interfaces/nilearn.py index 1d7bdff0e5..3492e2e233 100644 --- a/nipype/interfaces/nilearn.py +++ b/nipype/interfaces/nilearn.py @@ -106,7 +106,7 @@ def _run_interface(self, runtime): region_signals.astype(str))) # save output - self._results['out_file'] = os.path.join(runtime.cwd, self.inputs.out_file) + self._results['out_file'] = os.path.abspath(self.inputs.out_file) np.savetxt( self._results['out_file'], output, fmt=b'%s', delimiter='\t') return runtime From 8c61abf9dcb1b4ad8ae369884630f742919a465f Mon Sep 17 00:00:00 2001 From: oesteban Date: Fri, 19 Jan 2018 12:28:23 -0800 Subject: [PATCH 6/6] use runtime.prevcwd --- nipype/pipeline/engine/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nipype/pipeline/engine/utils.py b/nipype/pipeline/engine/utils.py index ec8d0e42ec..d34e2523fe 100644 --- a/nipype/pipeline/engine/utils.py +++ b/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', ''), } if hasattr(result.runtime, 'cmdline'):