GeezerVison is showing github.com — gh-101357: use `os.path.exists()` etc from pathlib by barneygale · Pull Request #101361 · python/cpython
Conversation
barneygale
commented
Jan 27, 2023
•
edited by mdboom
Loading
Uh oh!
There was an error while loading. Please reload this page.
The following pathlib.Path methods now directly call functions in os.path:
samefile()
exists()
is_dir()
is_file()
is_symlink()
In conjunction with #101324, this improves performance of these methods by using making fewer system calls than stat()
The original generic implementations of these methods are preserved in a new, internal _PathWithStat class. In future this may form the basis of a public AbstractPath class. The generic implementation of is_mount(), which was removed in 29650fea, is also restored to this class.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The following `pathlib.Path` methods now directly call functions in
`os.path`:
- `samefile()`
- `exists()`
- `is_dir()`
- `is_file()`
- `is_symlink()`
In conjunction with python#101324, this improves performance of these methods
by using making fewer system calls than `stat()`
The original generic implementations of these methods are preserved in a
new, internal `_PathWithStat` class. In future this may form the basis of
a public `AbstractPath` class. The generic implementation of `is_mount()`,
which was removed in 29650fea, is also restored to this class.
|
#101324 has landed. The general approach here makes sense. I imagine what we'll want to do is:
- add a
strict flag to the C functions
- add a Python wrapper around them for
os.path (since a new kwarg would be an API change that we probably want to avoid
- use them from here
Alternatively, we could make the C functions always raise the exceptions that pathlib raises, and the Python wrappers in os.path would catch those exceptions and return False. I think microbenchmarking is the only way to know if any of that matters. I think we will lose a little bit of the gains of #101324 either way, but that's ok as long as it's not significant, I guess.
|
barneygale reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
since a new kwarg would be an API change that we probably want to avoid
We added a similar strict keyword-only argument to os.path.realpath() recently - couldn't we do that here too?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I opened this PR mainly to demonstrate how to preserve pathlib's stat() helpers, but I don't have the time to bring it forward. Closing so that someone else might have a go.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.