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-68320, gh-88302 - Allow for pathlib.Path subclassing #31691

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Mar 5, 2022

Users may wish to define subclasses of pathlib.PurePath and Path to add or modify existing methods. Before this change, attempting to instantiate a subclass raised an exception like:

AttributeError: type object 'PPath' has no attribute '_flavour'

Previously the _flavour attribute was assigned as follows:

PurePath._flavour        = xxx not set!! xxx
PurePosixPath._flavour   = _PosixFlavour()
PureWindowsPath._flavour = _WindowsFlavour()

It's now set as follows:

PurePath._flavour        = os.path
PurePosixPath._flavour   = posixpath
PureWindowsPath._flavour = ntpath

Functionality from _PosixFlavour and _WindowsFlavour is moved into PurePath as underscored-prefixed classmethods. Flavour classes are removed.

A deeper dive into this patch can be read here: https://discuss.python.org/t/make-pathlib-extensible/3428/42

Fixes #68320 #88302

Users may wish to define subclasses of `pathlib.Path` to add or modify
existing methods. Before this change, attempting to instantiate a subclass
raised an exception like:

    AttributeError: type object 'PPath' has no attribute '_flavour'

Previously the `_flavour` attribute was assigned as follows:

    PurePath._flavour        = xxx not set!! xxx
    PurePosixPath._flavour   = _PosixFlavour()
    PureWindowsPath._flavour = _WindowsFlavour()

This commit replaces it with a `_pathmod` attribute, set as follows:

    PurePath._pathmod        = os.path
    PurePosixPath._pathmod   = posixpath
    PureWindowsPath._pathmod = ntpath

Functionality from `_PosixFlavour` and `_WindowsFlavour` is moved into
`PurePath` as underscored-prefixed classmethods. Flavours are removed.
Lib/pathlib.py Show resolved Hide resolved
@brettcannon brettcannon self-requested a review Mar 7, 2022
@barneygale
Copy link
Contributor Author

barneygale commented Apr 6, 2022

Hey @brettcannon, just bumping this PR in case it dropped off your radar.

@brettcannon
Copy link
Member

brettcannon commented Apr 7, 2022

@barneygale it hasn't, but my PR review queue is 11 PRs deep, this is in position 7, and I have worry about getting PEP 594 done in time for 3.11b1. Plus I have some stuff I need to get done for work this month which is eating into my paid OSS time.

@barneygale barneygale changed the title bpo-24132, bpo-44136 - Fix pathlib.Path subclassing. gh-68320, gh-88302 - Fix pathlib.Path subclassing. Apr 24, 2022
@brettcannon brettcannon reopened this Apr 27, 2022
@barneygale barneygale closed this Apr 29, 2022
@barneygale barneygale reopened this Apr 29, 2022
Copy link
Member

@brettcannon brettcannon left a comment

Most of the comments are either code touch-ups or asking to make the code more self-describing.

Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Outdated Show resolved Hide resolved
Lib/pathlib.py Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Apr 29, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

barneygale and others added 2 commits Apr 29, 2022
@bedevere-bot
Copy link

bedevere-bot commented Nov 15, 2022

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from brettcannon Nov 15, 2022
@brettcannon
Copy link
Member

brettcannon commented Nov 25, 2022

@barneygale now that #99031 is merged, let me know when you're ready for me to review this again.

Copy link
Member

@brettcannon brettcannon left a comment

Merge conflicts

@bedevere-bot
Copy link

bedevere-bot commented Nov 25, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@barneygale
Copy link
Contributor Author

barneygale commented Nov 25, 2022

I have made the requested changes; please review again

Thanks Brett :)

@bedevere-bot
Copy link

bedevere-bot commented Nov 25, 2022

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from brettcannon Nov 25, 2022
Copy link
Member

@brettcannon brettcannon left a comment

One small tweak to the tests, otherwise LGTM!

Lib/test/test_pathlib.py Show resolved Hide resolved
Lib/test/test_pathlib.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

bedevere-bot commented Dec 17, 2022

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@barneygale
Copy link
Contributor Author

barneygale commented Dec 17, 2022

I have made the requested changes; please review again

Thanks v much for all your help!

@bedevere-bot
Copy link

bedevere-bot commented Dec 17, 2022

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-bot bedevere-bot requested a review from brettcannon Dec 17, 2022
Lib/pathlib.py Show resolved Hide resolved
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.

Direct sub-classing of pathlib.Path