GH-106037: Disarm os.PathLike foot-shotgun in pathlib.PurePath user subclasses - #106043
GH-106037: Disarm os.PathLike foot-shotgun in pathlib.PurePath user subclasses#106043barneygale wants to merge 1 commit into
Conversation
Sorry, something went wrong.
…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.
AlexWaygood
left a comment
There was a problem hiding this comment.
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...?
Sorry, something went wrong.
|
That would at least allow me to set __fspath__ = None in AbstractPath, which would lessen the impact significantly.
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 |
Sorry, something went wrong.
|
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) |
Sorry, something went wrong.
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.