bpo-44136: remove pathlib._Flavour

[barneygale]

For bpo-24132, we'd like subclasses of AbstractPath to be able to customize flavour behaviour (like as_uri()) without needing to promote _Flavour to a public class.

(Copy-pasted bug description:)

Following #18841, #25264 and #25701, pathlib._Flavour and its subclasses are looking a bit pointless.

The implementations of is_reserved() and make_uri() (~as_uri()) can be readily moved to into PurePosixPath and PureWindowsPath, which removes some indirection. This follows the pattern of OS-specific stuff in PosixPath and WindowsPath.

The remaining methods, such as splitroot(), can be pulled into Pure*Path with an underscore prefix.

I'm generally a believer in composition over inheritance, but in this case _Flavour seems too small and too similar to PurePath to separate out into 3 extra classes.

There should be no impact on public APIs or performance.

I've tried to split the changes up across commits; it may be easier to browse the diff that way.

https://bugs.python.org/issue44136

[kfollstad]

@barneygale I thought your design was Path which inherits from AbstractPath which inherits from PurePath?

If that is the case, then what I don't understand about this is if you move all of the Windows flavour logic into PureWindowsPath, then how can someone who wants to inherit from AbstractPath do anything with Windows if they want to? There will be no Windows flavour and there will be no PureWindowsPath in the mro. I am misunderstanding something?

[barneygale]

I thought your design was Path which inherits from AbstractPath which inherits from PurePath?

This is correct!

how can someone who wants to inherit from AbstractPath do anything with Windows if they want to?

It depends what you mean by "do anything with Windows". The PureWindowsPath and WindowsPath classes function exactly as before.

If users want to subclass AbstractPath, but use Windows path syntax (drive letters etc), they can inherit from both AbstractPath and PureWindowsPath:

class MyWindowsLikePath(pathlib.AbstractPath, pathlib.PureWindowsPath):
    pass

Does that make sense?

[kfollstad]

No, sorry I definitely was not clear enough.

What I am saying is that you seem to be assuming the primacy of Posix. For example, how would I write a platform agnostic version of the following with your AbstractPath?

class TaggedPath(AbstractPath):
    def add_tag(self, tag):
        ...

or would it be:

class TaggedPath(AbstractPath, WindowsPurePath):
    def add_tag(self, tag):
        ...

Does it make sense what I am trying to point out?

The reason that Path isn't subclassable is that it is a factory which is dependent on its two subclasses, and upon instantiation a decision has to be made about what class to return, PosixPath or WindowsPath.

If I am understanding correctly it seems like your solution with removing flavour from AbstractPath ignores that Windows-syntax paths exist. If you ignore that Windows-syntax paths exist, then of course there is no decision to be made.

As such, to me this doesn't really seem to working towards solving bpo-24132 because if the user in the original post was just assuming a specific platform, they could have subclassed either PosixPath or WindowsPath and had no problems. The trick is to make it work while not assuming a specific platform. Maybe I am missing something, but I don't understand how that will be possible if you strip out the Windows-style path syntax into PureWindowsPath like you did in these commits.

[barneygale]

This is a very good point. My main use case here is not so much allowing customization of both WindowsPath and PosixPath from within a single class, but allowing entirely new kinds of Path-like paths to exist that might be backed by zip files, FTP servers, etc.

I'll consider this carefully. Making this MR draft for now!

[barneygale]

The most apparent question for me is: should we mandate explicit TaggedWindowsPath and TaggedPosixPath classes, similar to the WindowsPath and PosixPath classes in pathlib.py? User code would then look like:

class TaggedPath(Path):
    def __new__(cls, *args, **kwargs):
        ...  # return either TaggedWindowsPath or TaggedPosixPath

    def add_tag(self, tag):
        ...

class TaggedWindowsPath(TaggedPath, WindowsPath):
    pass

class TaggedPosixPath(TaggedPath, PosixPath):
    pass

If defining __new__ to return the right thing feels like boilerplate, we could add some sort of registration in __subclasshook__.

If having explicit TaggedWindowsPath and TaggedPosixPath types is itself too explicit/noisy, we're a bit stuffed because PosixPath and WindowsPath already work this way.

[barneygale]

And in fact, I don't think anything in my patch makes the situation better or worse for subclassing Path, but it does make things a little easier for subclassing AbstractPath (for embeded/remote filesystems)

[barneygale]

A bit more mucking around in the code. It's very possible to generalise Path.__new__, so I think we can support:

class TaggedPath(Path):
    def add_tag(self, tag):
        ...

@TaggedPath.register
class TaggedWindowsPath(TaggedPath, WindowsPath):
    pass

@TaggedPath.register
class TaggedPosixPath(TaggedPath, PosixPath):
    pass

i.e. no need to require users to write a __new__ function.

This PR is actually helps a little with the work, because we can have some notion of "not supported" on Path (+ subclasses) that's independent of flavour (which Path lacks entirely atm)

[kfollstad]

If having explicit TaggedWindowsPath and TaggedPosixPath types is itself too explicit/noisy, we're a bit stuffed because PosixPath and WindowsPath already work this way.

Yes, I agree. That Path/PosixPath/WindowsPath work that way is the origin of all of these complications.
I have also been challenged by this lack of subclassing and have been thinking about it and working on a PR for some time.

A while ago I recognized that the crux of this problems is the inverted dependency relationship that Path has with PosixPath and WindowsPath. Which by the way is why up until recently I viewed what you and I were working on as orthogonal - because you were reorganizing the internal components to create a new abstraction for Zip files, etc. However my issue was with seeing if reorganizing the factory structure was a workable idea. However, I think, unless I am mistaken, that everything that you are trying to accomplish with your AbstractPath also falls out of the new classes I defined.

However, that said it seems like there is still all sorts of issues that you have identified in internals that need work on. You definitely understand the nitty gritty of those far better than I do. Hopefully between the two of us we can work to make Pathlib a little more usable.

[barneygale]

Further discussion here: https://discuss.python.org/t/fixing-subclassing-in-pathlib/8983

I still support merging this PR.

[kfollstad]

And in fact, I don't think anything in my patch makes the situation better or worse for subclassing Path, but it does make things a little easier for sub classing AbstractPath (for embeded/remote filesystems)

Perhaps, but I think there are other ways of accomplishing what you are trying to do, without taking out flavour with these commits. In particular, by adding flavour (_generic_flavor = _PosixFlavour()) to your class at the time it is instantiated.

In this way, your AbstractPath could be implemented without having to rip out flavour. Path.__new__ will override this, preventing any problems with inheriting from AbstractPath.

If that is the case, the whole issue of how to solve subclassing in bpo-24132, that I am trying to solve with #26438 can be settled later, after all possibilities are considered. Perhaps there is a better way than what I proposed.

[barneygale]

Withdrawing this PR as I'm going to take a more piecemeal approach. Hopeful we can get rid of flavours without adding anything to PurePath + friends.