-
-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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-89812: Add pathlib._PathBase
#106337
GH-89812: Add pathlib._PathBase
#106337
Conversation
@brettcannon a long time ago in a PR far, far away, you said:
I couldn't make this work for So in this PR I've chosen However, I have a bunch of open questions in the PR description about how These questions make me think that Do you have any advice on the best way forward? Thanks! |
You have a couple of options:
|
I am unsure about the name VirtualPath! The way I’ve seen it used, a TarPath class, or SshPath, or S3Path would be examples of virtual filesystems: I give them paths that look like regular filesystem paths and they do custom operations to return info or data. Their base class is not itself a virtual path, it’s an (abstract?) base class! |
Fair point! How about |
Alternatively, |
Not a fan of deep modules, and even |
tarfile.TarPath
pathlib._PathBase
just stumbled uppon this and im wondering if a concept of a public Accessor of a path might make sense additionally it would be trivial to have "roots" at a path or a "root" with a directory file descriptor/network protocol |
We actually used to have accessor objects, but their interface was much the same as If we add accessors back in, most users would need to subclass two classes to implement their own path objects: the accessor class and the path class. The latter would be necessary for any user who wants to add their own path methods, for example. |
those "accessors" where nothing more than aliases, after all the implementation as done had no power over what the pure path portion can do with the real world a actualy "accessor" would be able to be tied to something like a directory file descriptor, or a path object, or a entirely different thing thats not a filesystem and yes there is some need for synchronization between concrete path capabilities and the accessors, however there is certain equivalence classes to keep in mind (Posix, Windows), so in a lot of cases my impression is that there will be more "accessors" than paths |
Intriguing. Could you sketch out the accessor API? |
(in the forums rather than a PR discussion please) |
i can dedicate some time to it later a basic concept of the accessor version ought to consider something like:
|
To summarise my thoughts on the above: in many cases this PR helps bring those things about, and I don't think it hinders any of those things. But let's chat more in a forum thread. What's left in this PR:
On the latter point, there are still two subtle and related problems:
Both of these issues could be solved if we can provide some way to
|
Bah, pushed a commit labelled "WIP". In fact it contains a solution to the above problem: stash a |
This will require further refactoring in another PR.
Hey @AA-Turner and @gst, thank you for your earlier reviews, very much appreciated. Please could you give it another pass, if you can spare the time? I feel OK about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just two notes. I think _split_stack
makes sense a method.
A
Thank you so much for the reviews, everyone :-) |
Congratulations Barney, a lot of hard work on your part paid off! A |
Add private `pathlib._PathBase` class. This will be used by an experimental PyPI package to incubate a `tarfile.TarPath` class. Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Add private
pathlib._PathBase
class. This will be used by an experimental PyPI package to incubate atarfile.TarPath
class.Implementation notes
In
pathlib.py
:Path
into_PathBase
andPath
_PathBase.is_junction()
andis_mount()
usingstat()
. In the latter case, restore the implementation deleted in gh-86943: implementpathlib.WindowsPath.is_mount()
#31458_PathBase.resolve()
usingabsolute()
andreadlink()
_PathBase._scandir()
usingiterdir()
In
test_pathlib.py
:PathBaseTest
to exercise the_PathBase
class directlyPathTest
intoDummyPathTest
andPathTest
. The former exercises a representative subclass of_PathBase
.DummyPathWithSymlinksTest
to exercise a similar subclass, this time with symlink support.Questionable bits
In
pathlib.py
, the_PathBase.is_junction()
and_PathBase.resolve()
methods are new code. The latter is very performance-sensitive and may need more work.Future work
pathlib._PathBase
public. There's still at least a couple of things to do first:MakePurePath._flavour
public_PathBase.__hash__()
,__eq__()
, and other comparison methods.