Skip to content

Conversation

@barneygale
Copy link
Contributor

@barneygalebarneygale commented Jun 23, 2023

We made it possible to subclass pathlib.PurePath in a68e585, which landed in 3.12. However, user subclasses automatically inherit an __fspath__() method, which may not be appropriate. For example, a user subclass may implement a "virtual" filesystem to provide access to a .zip file or FTP server. But it would be highly surprising if open(FTPPath(...)) attempted to open a local file.

This patch makes the os.PathLike interface opt-in for pathlib.PurePath subclasses. In pathlib itself, we opt into the os.PathLike interface for PurePosixPath, PureWindowsPath and Path. As PurePath is not instantiable (you always get a PurePosixPath or PureWindowsPath object back), this is backwards compatible with 3.11.

…os.PathLike` We made it possible to subclass `pathlib.PurePath` in a68e585, which landed in 3.12. However, user subclasses automatically inherit an `__fspath__()` method, which may not be appropriate. For example, a user subclass may implement a "virtual" filesystem to provide access to a `.zip` file or FTP server. But it would be highly surprising if `open(FTPPath(...))` attempted to open a *local* file. This patch makes the `os.PathLike` interface opt-in. In pathlib itself, we opt into the `os.PathLike` interface for `PurePosixPath`, `PureWindowsPath` and `Path`. As `PurePath` is not instantiable (you always get a `PurePosixPath` or `PureWindowsPath` object back), this is backwards compatible with 3.11, but not with earlier 3.12 betas.
Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

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

Nice idea. Are we sure it's backwards compatible to remove __fspath__ from PurePath? Looks like there are a few user subclasses of it in the wild: https://github.com/pytest-dev/pyfakefs/blob/b576d65bbd9a7508c90e6b00aa3b9eccf4a86fa1/pyfakefs/fake_pathlib.py#L796-L805

One possible alternative solution might be to implement #106046, and then just document that users should set __fspath__ to None if they don't want their path subclasses to be considered subtypes of os.PathLike...?

@barneygale
Copy link
ContributorAuthor

barneygale commented Jun 23, 2023

That would at least allow me to set __fspath__ = None in AbstractPath, which would lessen the impact significantly.

Looks like there are a few user subclasses of it in the wild

That example you gave was only recently updated for 3.12 beta 1. There shouldn't be many more out there, and as I consider this a bugfix, it's reasonable to change behaviour anyway I think

@barneygale
Copy link
ContributorAuthor

Alex, I've played around with just setting __fspath__ = None where I need, and it works nicely. I mean, the exception message is bogus, but that's still a simpler fix than this PR or the previous _BasePurePath PR. Thanks so much for the pointer. I'll close this but keep the bug open (it's technically valid, but I have a workaround for AbstractPath so less pressing)

Sign up for freeto 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

@barneygale@AlexWaygood@bedevere-bot