| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@brettcannon a long time ago in a PR far, far away, you said:
I couldn't make this work for zipfile.Path due to a couple of fundamental differences in the interface, e.g. the root attribute stores a string in pathlib.PurePath, but a ZipFile instance in zipfile.Path. It's possible to make it work with a couple of deprecations/removals, or an entirely new ZipPath class. So in this PR I've chosen tarfile.TarPath as the initial target. I think it does a good job of showing the viability of the pathlib._VirtualPath interface. However, I have a bunch of open questions in the PR description about how TarPath itself should work. It's not as straightforward as .zip files, because tarballs support symlinks and hardlinks, and because tarfile.TarFile doesn't support reading and writing the same file. These questions make me think that tarfile.TarPath should first be gestated in a PyPI package before we add it to the standard library. But that leaves a bit of a chicken-and-egg problem with getting pathlib._VirtualPath in. Do you have any advice on the best way forward? Thanks! |
Sorry, something went wrong.
You have a couple of options:
|
Sorry, something went wrong.
|
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! |
Sorry, something went wrong.
|
Fair point! How about PathBase? It squares with the names of base classes in io, I think. |
Sorry, something went wrong.
|
Alternatively, pathlib.abc.VirtualPath? |
Sorry, something went wrong.
|
Not a fan of deep modules, and even .abc.VirtualPath doesn’t seem great to me. |
Sorry, something went wrong.
We actually used to have accessor objects, but their interface was much the same as Path objects, and so they amounted to nothing more than a layer of indirection. They were removed in #25701. 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. |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
|
Intriguing. Could you sketch out the accessor API? |
Sorry, something went wrong.
|
(in the forums rather than a PR discussion please) |
Sorry, something went wrong.
|
i can dedicate some time to it later a basic concept of the accessor version ought to consider something like:
|
Sorry, something went wrong.
|
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 stat() a path without resolving any symlinks. The resolve() method would call this stat() method first, and only call readlink() if the stat result indicates a symlink. A couple of options spring to mind:
|
Sorry, something went wrong.
|
Bah, pushed a commit labelled "WIP". In fact it contains a solution to the above problem: stash a _resolving flag on path objects, which means resolve() can safely call stat() and readlink() without hitting infinite recursion. No weird APIs for users. |
Sorry, something went wrong.
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 _PathBase.resolve(): it's entirely private for the timebeing, and I intend to add further tests and optimization before we drop the underscore from _PathBase, so if we can't spot any clear defects I suspect it's fine to land? Grateful for your guidance! |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks! Just two notes. I think _split_stack makes sense a method.
A
Sorry, something went wrong.
|
Thank you so much for the reviews, everyone :-) |
Sorry, something went wrong.
|
Congratulations Barney, a lot of hard work on your part paid off! A |
Sorry, something went wrong.
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>
| Back | FazBrowse Home | New Git URL |
Add private pathlib._PathBase class. This will be used by an experimental PyPI package to incubate a tarfile.TarPath class.
Implementation notes
In pathlib.py:
In test_pathlib.py:
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