Skip to content

Conversation

@barneygale
Copy link
Contributor

@barneygalebarneygale commented May 23, 2023

Move __fspath__(), __bytes__() and as_uri() methods from PurePath to a new _PurePathExt subclass. This new subclass is inherited by PurePosixPath, PureWindowsPath and Path.

Because PurePath isn't directly instantiatable (you get a PurePosixPath or PureWindowsPath instance back), this shouldn't change user-facing behaviour.

The methods must not be inherited future tarfile.TarPath and pathlib.AbstractPath classes, which will subclass PurePath.

This internal class excludes the `__fspath__()`, `__bytes__()` and `as_uri()` methods, which must not be inherited by a future `tarfile.TarPath` class.
@barneygalebarneygale changed the title GH-89812: Add pathlib._LexicalPathGH-89812: Add pathlib._BasePurePathMay 23, 2023
@barneygale
Copy link
ContributorAuthor

The test naming is pretty janky. #104829 will help.

barneygaleand others added 2 commits May 24, 2023 16:04
Co-authored-by: Éric <merwok@netwok.org>
barneygaleand others added 2 commits May 24, 2023 17:36
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@barneygale
Copy link
ContributorAuthor

Would anyone be willing to review #104829? It blocks this PR. (please and thank you!)

@barneygalebarneygale marked this pull request as draft June 7, 2023 20:26
@barneygalebarneygale marked this pull request as ready for review June 14, 2023 16:12
@barneygale
Copy link
ContributorAuthor

#104829 has landed, so test naming should be much improved! Think this is ready for review again.

@barneygalebarneygale changed the title GH-89812: Add pathlib._BasePurePathGH-89812: Add pathlib._PurePathExtJun 22, 2023
@barneygale
Copy link
ContributorAuthor

barneygale commented Jun 22, 2023

I've revised the implementation such that the methods are actually removed from PurePath. This will allow our future AbstractPath class to inherit PurePath, as it should conceptually:

image

(Contrast this with the earlier proposed hierarchy where PurePath and AbstractPath are siblings.

The methods are moved to a new _PurePathExt class, which is inherited by PurePosixPath, PureWindowsPath and Path.

This change will not be visible to users who try to instantiate PurePath, because they'll always get a PurePosixPath or PureWindowsPath instance back.

It will be visible to users who subclass PurePath, but that has only just become possible in 3.12. As such I think it's worth backporting this into 3.12 for consistency.

@barneygale
Copy link
ContributorAuthor

barneygale commented Jun 22, 2023

Here's a version of the diagram that includes the private _PurePathExt class. The diagram in the post above excluded it in order to show the public view of the class hierarchy.

image

@merwok
Copy link
Member

Just a note that I’m meditating on https://hynek.me/articles/python-subclassing-redux/ and linked articles at the moment and wondering if controlling subclass explosion in pathlib should be a guideline 🙂

@barneygale
Copy link
ContributorAuthor

barneygale commented Jun 23, 2023

Oh I totally agree. I'd prefer to add onlypathlib.AbstractPath, but I'm prevented by the fact that PurePath has an __fspath__() method :)

And if I were to do pathlib over again, I'd scrap the *WindowsPath and *PosixPath classes. That could be controlled by an argument to the PurePath initialiser instead. Oh well...

@barneygalebarneygale marked this pull request as draft June 23, 2023 11:57
@barneygale
Copy link
ContributorAuthor

I've converted this back to a draft as I'm still in two minds about how to implement this. It's possible I'll add _AbstractPath into this PR, or close this PR and log another, or something else. Sorry for the noise

@barneygale
Copy link
ContributorAuthor

Friendship ended with #104810, now #106043 is my best friend.

@CAM-Gerlach
Copy link
Member

@barneygale

image

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.

6 participants

@barneygale@merwok@CAM-Gerlach@brettcannon@AlexWaygood@bedevere-bot