RF: Provide functions to augment old Path.mkdir, Path.resolve methods #3050
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #3050 +/- ##
==========================================
- Coverage 67.03% 66.61% -0.42%
==========================================
Files 343 343
Lines 44077 44065 -12
Branches 5551 5549 -2
==========================================
- Hits 29545 29355 -190
- Misses 13775 13917 +142
- Partials 757 793 +36
Continue to review full report at Codecov.
|
effigies
force-pushed
the
fix/pathlib_patch
branch
from
d729a11
to
6d4e821
Compare
effigies
force-pushed
the
fix/pathlib_patch
branch
from
6d4e821
to
9eefdcd
Compare
|
A review would be appreciated. This fixes one of the two issues in #3046. |
oesteban
reviewed
Sep 27, 2019
| except TypeError: # PY35 | ||
| pass | ||
|
|
||
| path = path.absolute() |
There was a problem hiding this comment.
Is this method documented? I had to go to the code to find it.
There was a problem hiding this comment.
$ 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?
There was a problem hiding this comment.
Cool. I did check that it exists in py35.
| def _resolve_with_filenotfound(path, **kwargs): | ||
| """ Raise FileNotFoundError instead of OSError """ | ||
| try: | ||
| return path.resolve(**kwargs) |
There was a problem hiding this comment.
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'
There was a problem hiding this comment.
It is. It's a helper function to promote OSErrors to FileNotFoundErrors, and lets all others get caught higher.
There was a problem hiding this comment.
Gotcha, yes, I see the TypeError in path_resolve. Thanks!
oesteban
approved these changes
Sep 27, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In #3046, we see that pytest using our monkey-patched
Pathobjects. This seems generally unsafe, and evidently possible for us to screw up, so I've switched to functions that try to use the original methods, with fallback implementations when they fail. These are marked with#PY*tags so we can drop them when we drop support for those versions.I've left the
Path.write_textmethod for now, just because the logic there is simpler and we'll be dropping Python 3.4 very soon.Alternative to #3048.
List of changes proposed in this PR (pull-request)
Path.mkdirmonkeypatch withnipype.utils.filemanip.path_mkdirPath.resolvemonkeypatch withnipype.utils.filemanip.path_resolveAcknowledgment