Skip to content

RF: Provide functions to augment old Path.mkdir, Path.resolve methods #3050

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.

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
Sep 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions nipype/interfaces/base/traits_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

from traits.api import Unicode
from future import standard_library
from ...utils.filemanip import Path, USING_PATHLIB2
from ...utils.filemanip import Path, USING_PATHLIB2, path_resolve

if USING_PATHLIB2:
from future.types.newstr import newstr
Expand Down Expand Up @@ -147,7 +147,7 @@ def validate(self, objekt, name, value, return_pathlike=False):
self.error(objekt, name, str(value))

if self.resolve:
value = value.resolve(strict=self.exists)
value = path_resolve(value, strict=self.exists)

if not return_pathlike:
value = str(value)
Expand Down
3 changes: 2 additions & 1 deletion nipype/pipeline/engine/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from ... import logging, config, LooseVersion
from ...utils.filemanip import (
Path,
path_mkdir,
indirectory,
relpath,
makedirs,
Expand Down Expand Up @@ -123,7 +124,7 @@ def write_node_report(node, result=None, is_mapnode=False):

cwd = node.output_dir()
report_file = Path(cwd) / '_report' / 'report.rst'
report_file.parent.mkdir(exist_ok=True, parents=True)
path_mkdir(report_file.parent, exist_ok=True, parents=True)

lines = [
write_rst_header('Node: %s' % get_print_name(node), level=0),
Expand Down
73 changes: 34 additions & 39 deletions nipype/utils/filemanip.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,34 +59,42 @@ def __init__(self, path):
from pathlib2 import Path
USING_PATHLIB2 = True

try: # PY35 - strict mode was added in 3.6
Path('/invented/file/path').resolve(strict=True)
except TypeError:
def _patch_resolve(self, strict=False):
"""Add the argument strict to signature in Python>3,<3.6."""
resolved = Path().old_resolve() / self

if strict and not resolved.exists():
raise FileNotFoundError(resolved)
return resolved

Path.old_resolve = Path.resolve
Path.resolve = _patch_resolve
except FileNotFoundError:
pass
except OSError:
# PY2
def _patch_resolve(self, strict=False):
"""Raise FileNotFoundError instead of OSError with pathlib2."""
try:
resolved = self.old_resolve(strict=strict)
except OSError:
raise FileNotFoundError(self.old_resolve())

return resolved
def _resolve_with_filenotfound(path, **kwargs):
""" Raise FileNotFoundError instead of OSError """
try:
return path.resolve(**kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this is intended (PY35):

Python 3.5.5 | packaged by conda-forge | (default, Jul 23 2018, 23:45:43) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.6.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: %load_ext autoreload 
   ...: %autoreload 2                                                                                                                                                                                                                                                                                                                                                                                              

In [2]: from nipype.utils import filemanip as fm                                                                                                                                                                                                                                                                                                                                                                   
190927-11:57:03,978 nipype.utils INFO:
	 Running nipype version 1.3.0-dev+g9eefdcdb1 (latest: 1.2.3)

In [3]: from pathlib import Path                                                                                                                                                                                                                                                                                                                                                                                   

In [4]: fm._resolve_with_filenotfound(Path('/some/made/out/path'), strict=False)                                                                                                                                                                                                                                                                                                                                   
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-b819f7473f4f> in <module>
----> 1 fm._resolve_with_filenotfound(Path('/some/made/out/path'), strict=False)

~/workspace/nipype/nipype/utils/filemanip.py in _resolve_with_filenotfound(path, **kwargs)
     64     """ Raise FileNotFoundError instead of OSError """
     65     try:
---> 66         return path.resolve(**kwargs)
     67     except OSError as e:
     68         if isinstance(e, FileNotFoundError):

TypeError: resolve() got an unexpected keyword argument 'strict'

Copy link
Member Author

Choose a reason for hiding this comment

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

It is. It's a helper function to promote OSErrors to FileNotFoundErrors, and lets all others get caught higher.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, yes, I see the TypeError in path_resolve. Thanks!

except OSError as e:
if isinstance(e, FileNotFoundError):
raise
raise FileNotFoundError(str(path))


def path_resolve(path, strict=False):
try:
return _resolve_with_filenotfound(path, strict=strict)
except TypeError: # PY35
pass

path = path.absolute()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method documented? I had to go to the code to find it.

Copy link
Member Author

Choose a reason for hiding this comment

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

$ pydoc pathlib.Path.absolute | cat
Help on function absolute in pathlib.Path:

pathlib.Path.absolute = absolute(self)
    Return an absolute version of this path.  This function works
    even if the path doesn't point to anything.
    
    No normalization is done, i.e. all '.' and '..' will be kept along.
    Use resolve() to get the canonical path to a file.

Yeah, it looks like it's not on the web docs. Is that a problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

nono, just confirming.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. I did check that it exists in py35.

if strict or path.exists():
return _resolve_with_filenotfound(path)

# This is a hacky shortcut, using path.absolute() unmodified
# In cases where the existing part of the path contains a
# symlink, different results will be produced
return path


def path_mkdir(path, mode=0o777, parents=False, exist_ok=False):
try:
return path.mkdir(mode=mode, parents=parents, exist_ok=exist_ok)
except TypeError: # PY27/PY34
if parents:
return makedirs(str(path), mode=mode, exist_ok=exist_ok)
elif not exist_ok or not path.exists():
return os.mkdir(str(path), mode=mode)

Path.old_resolve = Path.resolve
Path.resolve = _patch_resolve

if not hasattr(Path, 'write_text'):
# PY34 - Path does not have write_text
Expand All @@ -95,19 +103,6 @@ def _write_text(self, text):
f.write(text)
Path.write_text = _write_text

try: # PY27/PY34 - mkdir does not have exist_ok
from .tmpdirs import TemporaryDirectory
with TemporaryDirectory() as tmpdir:
(Path(tmpdir) / 'exist_ok_test').mkdir(exist_ok=True)
except TypeError:
def _mkdir(self, mode=0o777, parents=False, exist_ok=False):
if parents:
makedirs(str(self), mode=mode, exist_ok=exist_ok)
elif not exist_ok or not self.exists():
os.mkdir(str(self), mode=mode)

Path.mkdir = _mkdir


def split_filename(fname):
"""Split a filename into parts: path, base filename and extension.
Expand Down
27 changes: 16 additions & 11 deletions nipype/utils/tests/test_filemanip.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
check_forhash, _parse_mount_table, _cifs_table, on_cifs, copyfile,
copyfiles, ensure_list, simplify_list, check_depends,
split_filename, get_related_files, indirectory,
loadpkl, loadcrash, savepkl, FileNotFoundError, Path)
loadpkl, loadcrash, savepkl, FileNotFoundError, Path,
path_mkdir, path_resolve)


def _ignore_atime(stat):
Expand Down Expand Up @@ -572,21 +573,24 @@ def test_unversioned_pklization(tmpdir):
loadpkl('./pickled.pkz')


def test_Path_strict_resolve(tmpdir):
def test_path_strict_resolve(tmpdir):
"""Check the monkeypatch to test strict resolution of Path."""
tmpdir.chdir()

# Default strict=False should work out out of the box
testfile = Path('somefile.txt')
assert '%s/somefile.txt' % tmpdir == '%s' % testfile.resolve()
resolved = '%s/somefile.txt' % tmpdir
assert str(path_resolve(testfile)) == resolved
# Strict keyword is always allowed
assert str(path_resolve(testfile, strict=False)) == resolved

# Switching to strict=True must raise FileNotFoundError (also in Python2)
with pytest.raises(FileNotFoundError):
testfile.resolve(strict=True)
path_resolve(testfile, strict=True)

# If the file is created, it should not raise
open('somefile.txt', 'w').close()
assert '%s/somefile.txt' % tmpdir == '%s' % testfile.resolve(strict=True)
assert str(path_resolve(testfile, strict=True)) == resolved


@pytest.mark.parametrize("save_versioning", [True, False])
Expand All @@ -598,17 +602,18 @@ def test_pickle(tmp_path, save_versioning):
assert outobj == testobj


def test_Path(tmpdir):
def test_path_mkdir(tmpdir):
tmp_path = Path(tmpdir.strpath)

(tmp_path / 'textfile').write_text('some text')
# PY34: Leave as monkey-patch
Path.write_text(tmp_path / 'textfile', 'some text')

with pytest.raises(OSError):
(tmp_path / 'no' / 'parents').mkdir(parents=False)
path_mkdir(tmp_path / 'no' / 'parents', parents=False)

(tmp_path / 'no' / 'parents').mkdir(parents=True)
path_mkdir(tmp_path / 'no' / 'parents', parents=True)

with pytest.raises(OSError):
(tmp_path / 'no' / 'parents').mkdir(parents=False)
path_mkdir(tmp_path / 'no' / 'parents', parents=False)

(tmp_path / 'no' / 'parents').mkdir(parents=True, exist_ok=True)
path_mkdir(tmp_path / 'no' / 'parents', parents=True, exist_ok=True)