gh-68320, gh-88302 - Allow for pathlib.Path subclassing

[barneygale]

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

• Issue: Direct sub-classing of pathlib.Path #68320

[barneygale]

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

[brettcannon]

@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.

[brettcannon]

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

[bedevere-bot]

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.

[brettcannon]

Thanks for taking the time to see this through, @barneygale !

[barneygale]

Thank you so much for your help with this Brett! Hope you have a restful break :)

[zmievsa]

@barneygale Congrats! You started so long ago but we finally get the results. It's an amazing effort.

[barneygale]

Hi mzipay. Your code has a couple of problems:

• ('extslot') does not define a tuple, and generally you don't need to worry about __slots__ unless performance is a concern
• Your initialiser assigns to _extslot with an underscore prefix, but the rest of your code doesn't use the prefix
• Your initialiser doesn't call super().

Here's a fixed example:

>>> from pathlib import Path
>>> 
>>> 
>>> class ExtPath(Path):
...     def __init__(self, *args):
...         self.extslot = 'this will disappear'
...         super().__init__(*args)
... 
>>> 
>>> extpath = ExtPath('Spam/eggs.txt')
>>> extpath.extslot
'this will disappear'
>>> extpath.parent.extslot
'this will disappear'

Hope that helps. If you have any specific criticisms or questions you have about pathlib's design I'll do my best to address them. But bear in mind you'll catch more flies with honey than vinegar ;)

[zmievsa]

@mzipay Pep 20 is not "Python principles". It's poetry.

https://mail.python.org/archives/list/[email protected]/message/WG5VOX4R4WS5MHUR5NTF4IL3BKF4UI4Y/

[mzipay]

@barneygale Apologies, (1) I was trying to simplify my sample code and didn't copy/paste it properly. (2) No "vinegar" intended, just trying to be succinct. (In fact I intentionally noted that "I love the idea of pathlib. I use it as often as I possibly can.")

If you have any specific criticisms or questions you have about pathlib's design I'll do my best to address them.

Yes, I can be more specific: pathlib's __new__/__init__ behavior is unexpected and still "breaks" subclassing. My example code very clearly demonstrates that, and I'll include it in its entirety below.
(I don't necessarily think the Path subclassing needs to be fixed. It's be great if it did actually get fixed, but at least a prominent disclaimer of "not intended to be subclassed" at the top of the module's doc page would be appropriate.)

First, though:

generally you don't need to worry about slots unless performance is a concern

In my real-world use case, I am dealing with tens of thousands of Paths, so performance is a concern for me.
But even if it weren't, it doesn't change that the pathlib behavior is unexpected. Any state that is managed in a Path subclass's __new__ or __init__ gets lost, as you will see below.
To be more clear, I'll even remove my use of __slots__ for the time being - the example will still fail.

Now back to the proposed fix (which doesn't work). Actually run this on 3.12.0a6:

from pathlib import Path

class ExtPath(Path):

    def __init__(self, *args):
        super().__init__(*args)
        self.value = 'this will disappear'

if __name__ == '__main__':
    extpath = ExtPath('Spam/eggs.txt')
    assert extpath.value == 'this will disappear'
    assert type(extpath.parent) is ExtPath
    assert extpath.parent.value == 'this will disappear'

You'll get TypeError: object.__init__() takes exactly one argument (the instance to initialize). This is because pathlib's __new__ impls are not consistent w/r/t __init__. (Neither Path nor PurePath even define __init__, so it's object.__init__ that would receive the proposed "fix" - which is why it doesn't work.)

Even if the super call is corrected, it still produces the unexpected result from my initial posting (granted I should've just posted the full example in the first place):

from pathlib import Path

class ExtPath(Path):

    def __init__(self, *args):
        super().__init__()  # object.__init__
        self.value = 'this will disappear'

if __name__ == '__main__':
    extpath = ExtPath('Spam/eggs.txt')
    assert extpath.value == 'this will disappear'
    assert type(extpath.parent) is ExtPath
    assert extpath.parent.value == 'this will disappear'

Running this results in an unexpected AttributeError:

$ python extpath.py 
Traceback (most recent call last):
  File "/tmp/extpath.py", line 13, in <module>
    assert extpath.parent.value == 'this will disappear'
           ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'ExtPath' object has no attribute 'value'

Why does this happen?

Take a close look at 3.12.0a6's pathlib.py, lines 302-317. Here you will find the PurePath._from_parts and PurePath._from_parsed_parts methods.

These methods are used, from what I gather inspecting the code, to offer performance improvements by avoiding the need to re-parse the path segments. But they do so by bypassing __new__ and __init__ altogether!

Note the use of object.__new__(cls) in these methods to create the objects. As a result, neither PurePath.__new__ nor Path.__new__ are invoked (same for __init__s, even if they were defined).

This means that any property or method of (a subclass of) Path that returns a new Path (like .parent in my example) will return what seems to be an object of the correct type (note my second assert), but any state that that subclass manages, either in its own __new__ or __init__, is lost; hence the AttributeError.

[mzipay]

Seems this just got fixed in another issue (#85281) and merged a few days ago. Built 3.12.0a7, subclassing Path now appears to have expected behavior.

[barneygale]

Yep that's been a longstanding pathlib issue; it was difficult to address without regressing performance, and so there was a lot of preparatory work involved first. Glad to hear you have it working now!