GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-24132: Direct sub-classing of pathlib.Path by kfollstad · Pull Request #26438 · python/cpython

Conversation

@kfollstad

@kfollstad kfollstad commented May 28, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Updated based on feedback - currently marked as draft

I submit for your consideration what I believe is the first working version (with documentation) of an extensible, subclassable PurePath/Path to close bpo-24132, a 6-year-old bug. I have also posted a more detailed version of this explanation on discuss.python.org because this fix involves adding classes to pathlib and understanding why that is required takes a significant explanation.

These classes are a complete set of alternatives to PurePath and Path which omit the factory design but instead attach the flavour to the class at the time of instantiation. These newly designed classes still pass all of the test cases that are run against PurePath and Path on both Windows and Linux. Because they omit the factory design, they don't have inverted dependencies and therefore are naively subclassable and extensible in any way you see fit.

To this end I give you:

SimplePath (subclassable alternative to PurePath)
SimplePosixPath (subclassable alternative to PurePosixPath)
SimpleWindowsPath (subclassable alternative to PureWindowsPath)
FilePath (subclassable alternative to Path/PosixPath/WindowsPath)

On Windows, SimplePath behaves as if it were PureWindowsPath. On Posix, it behaves as PurePosixPath. Similarly, FilePath behaves like WindowsPath on Windows and PosixPath on Posix. These four classes combined could, if one were so inclined, act as a complete replacement for the existing six Path/PurePath classes.

Despite being 11 commits, it's essentially just two refactors. The first splits the two responsibilities PurePath has, separating the base class methods into _PurePathBase. The second does the same with Path, moving the base class methods into PathIOMixin. The other commits are just minor tweaks and documentation to account for all of that.

https://bugs.python.org/issue24132

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
kfollstad added 11 commits May 28, 2021 14:14
@kfollstad
bpo-24132: Add stub SimplePath/FilePath classes to pathlib
f011491
Introduce stub classes with initial tests on what will become
subclassable alternatives to PurePath/Path that do not have
inverted dependencies.
@kfollstad
bpo-24132: Replace hardcoded references where possible in pathlib
ff1d455
Replace references in PurePath and changed function in
_BasePurePathTest.test_ordering_common to test missing execution paths.
This is preparation for seperating the base class and factory
functionality in PurePath in order to fully implement SimplePath.
@kfollstad
bpo-24132: Refactor PurePath methods into base class in pathlib
488994f
Introduce new base class _PurePathBase and move all of the methods
from PurePath into it (except for the factory __new__) for use in
building SimplePath / FilePath.
@kfollstad
bpo-24132: Completed SimplePath in pathlib
6a69e0d
SimplePath provides a subclassable alternative to PurePath which is
otherwise functionally equivalent -- passing all of the existing
PurePath tests.
@kfollstad
bpo-24132: Partial implmentation of FilePath in pathlib
09317e2
Partial implementation of FilePath which offers all of the PurePath
and SimplePath functionality and passes the equivalent existing tests.
@kfollstad
bpo-24132: Fix inconsistent organization of is_mount in pathlib
d966acf
Existing is_mount method in Path was Posix only and not cross
platform even though it was in a cross-platform base class. The existing
design relied upon being overriden later in WindowsPath. Consolidated
code from both into new Path.is_mount() which is now cross plaform
similiar to all of the other Path methods.
@kfollstad
bpo-24132: Replace additional hardcoded references in pathlib
ed5f237
Replace hardcorded references to pathlib.Path so that the base
class and factory functionality of Path can be seperated.
@kfollstad
bpo-24132: Completed FilePath in pathlib
ecc341b
FilePath provides a subclassable alternative to Path which is
otherwise functionally equivalent -- passing all of the existing
Path tests.
@kfollstad
bpo-24123: Add SimplePosixPath and SimpleWindowsPath to pathlib
2700e46
SimplePosixPath and SimpleWindowsPath provide a subclassable
alternative to PurePosixPath and PureWindowsPath which are otherwise
functionally equivalent -- passing all of the existing PurePosixPath
and PureWindowsPath tests.
@kfollstad
bpo-24132: Fix validity of inter-path comparison in pathlib
d15d8da
Fix _BasePurePath comparison operators so that all of the new
path classes in pathlib are comparable with one another and return
results that are consistent with the existing behavoir, i.e.
Path('/') == PurePath('/').
@kfollstad
bpo-24132: Document FilePath and SimplePath family in pathlib
6028b6d
Document and make public FilePath, SimplePath, SimplePosixPath,
SimpleWindowsPath, and PathIOMixin. Also add section to pathlib
documentation regarding the limitations of PurePath and Path with
regards to subclassing and how to use these new classes as alternatives.
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 28, 2021
@bedevere-bot bedevere-bot added the awaiting review label May 28, 2021
@barneygale

barneygale commented May 28, 2021

Copy link
Copy Markdown
Contributor

On Windows, SimplePath behaves as if it were PureWindowsPath. On Posix, it behaves as PurePosixPath. Similarly, FilePath behaves like WindowsPath on Windows and PosixPath on Posix. These four classes combined (less PathIOMixin) could, if one were so inclined, act as a complete replacement for the existing six Path/PurePath classes.

This is a key design decision - should we depart from pathlib's existing Path, WindowsPath and PosixPath abstraction and handle both systems in a single type?

My personal view is that this is just too large a departure from how pathlib works right now. It's too hard to mentally model and explain. You and I only understand it because we've had our noses in pathlib for too long! If I could go back in time and convince Antoine to consider using Path for all kinds of concrete path I would, that ship has already sailed in my view.

I don't think this needs 5 new classes to implement. We have options for how to make creating a trio of MyPath, MyPosixPath and MyWindowsPath pretty painless.

I also have an incurable phobia of mixin classes :(

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kfollstad kfollstad mentioned this pull request May 29, 2021
bpo-44136: remove pathlib._Flavour #26141
Closed
@kfollstad
kfollstad marked this pull request as draft May 31, 2021 05:01
@kfollstad

kfollstad commented May 31, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@barneygale, thank you very much for taking the time to review my PR / ideas thread. I know you have spent a lot of time wrestling with pathlib which makes your input all the more valuable. I've marked this draft while address some of the points you made.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kfollstad

kfollstad commented Jun 25, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Closing this and replacing it with an alternative approach to solving bpo-24132 via #26906 which makes no changes to the API.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@kfollstad kfollstad closed this Jun 25, 2021
@projetmbc projetmbc mannequin mentioned this pull request Jun 21, 2022
Direct sub-classing of pathlib.Path #68320
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@barneygale barneygale Awaiting requested review from barneygale barneygale will be requested when the pull request is marked ready for review barneygale is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@kfollstad @barneygale @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.