Merge pull request #3050 from effigies/fix/pathlib_patch

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

Author: effigies

nipype/interfaces/base/traits_extension.py

@@ -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
@@ -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)

nipype/pipeline/engine/utils.py

@@ -25,6 +25,7 @@
 from ... import logging, config, LooseVersion
 from ...utils.filemanip import (
     Path,
+    path_mkdir,
     indirectory,
     relpath,
     makedirs,
@@ -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),

nipype/utils/filemanip.py

@@ -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)
+    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()
+    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
@@ -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.

nipype/utils/tests/test_filemanip.py

@@ -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):
@@ -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])
@@ -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)