Skip to content

FIX: Build cmdline from working directory #2521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2018
Merged

FIX: Build cmdline from working directory #2521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c10c92
FIX: Build cmdline from working directory
effigies Mar 30, 2018
555a905
FIX: Revert directory when exception raised
effigies Mar 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions nipype/pipeline/engine/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from ...utils.filemanip import (md5, FileNotFoundError, filename_to_list,
list_to_filename, copyfiles, fnames_presuffix,
loadpkl, split_filename, load_json, makedirs,
emptydirs, savepkl, to_str)
emptydirs, savepkl, to_str, indirectory)

from ...interfaces.base import (traits, InputMultiPath, CommandLine, Undefined,
DynamicTraitedSpec, Bunch, InterfaceResult,
Expand Down Expand Up @@ -627,7 +627,8 @@ def _run_command(self, execute, copyfiles=True):
self._interface.__class__.__name__)
if issubclass(self._interface.__class__, CommandLine):
try:
cmd = self._interface.cmdline
with indirectory(outdir):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does outdir always exist at this point?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We create the command.txt file in line 637, just below here, and copy files to working directory just above.

A quick scan didn't show me where the directory is created, but I'm reasonable confident it exists.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The directory is created here, inside run():

https://github.com/effigies/nipype/blob/555a905f623be290a16c5fede1ea3a1558ac1f57/nipype/pipeline/engine/nodes.py#L469

Therefore, it is not granted that it will exist at this point. We could add a makedirs call to the context manager.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run() then calls _run_interface(), which calls _run_command(), which contains this code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but you can always call cmdline manually before run.

inside a workflow this does not apply, but many people use the interfaces and call cmdline in their ipython interactive sessions.

cmd = self._interface.cmdline
except Exception as msg:
result.runtime.stderr = '{}\n\n{}'.format(
getattr(result.runtime, 'stderr', ''), msg)
Expand Down
11 changes: 11 additions & 0 deletions nipype/utils/filemanip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import os.path as op
import re
import shutil
import contextlib
import posixpath
import simplejson as json
import numpy as np
Expand Down Expand Up @@ -916,3 +917,13 @@ def relpath(path, start=None):
if not rel_list:
return os.curdir
return op.join(*rel_list)


@contextlib.contextmanager
def indirectory(path):
cwd = os.getcwd()
os.chdir(path)
try:
yield
finally:
os.chdir(cwd)
33 changes: 32 additions & 1 deletion nipype/utils/tests/test_filemanip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
save_json, load_json, fname_presuffix, fnames_presuffix, hash_rename,
check_forhash, _parse_mount_table, _cifs_table, on_cifs, copyfile,
copyfiles, filename_to_list, list_to_filename, check_depends,
split_filename, get_related_files)
split_filename, get_related_files, indirectory)


def _ignore_atime(stat):
Expand Down Expand Up @@ -490,3 +490,34 @@ def test_cifs_check():

_cifs_table[:] = []
_cifs_table.extend(orig_table)


def test_indirectory(tmpdir):
tmpdir.chdir()

os.makedirs('subdir1/subdir2')
sd1 = os.path.abspath('subdir1')
sd2 = os.path.abspath('subdir1/subdir2')

assert os.getcwd() == tmpdir.strpath
with indirectory('/'):
assert os.getcwd() == '/'
assert os.getcwd() == tmpdir.strpath
with indirectory('subdir1'):
assert os.getcwd() == sd1
with indirectory('subdir2'):
assert os.getcwd() == sd2
with indirectory('..'):
assert os.getcwd() == sd1
with indirectory('/'):
assert os.getcwd() == '/'
assert os.getcwd() == sd1
assert os.getcwd() == sd2
assert os.getcwd() == sd1
assert os.getcwd() == tmpdir.strpath
try:
with indirectory('subdir1'):
raise ValueError("Erroring out of context")
except ValueError:
pass
assert os.getcwd() == tmpdir.strpath