Skip to content
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

gh-101357: use os.path.exists() etc from pathlib #101361

Closed

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Jan 27, 2023

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.


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.
@mdboom
Copy link
Contributor

mdboom commented Feb 8, 2023

#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
Copy link
Contributor Author

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?

@barneygale
Copy link
Contributor Author

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.

@barneygale barneygale closed this Feb 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants