-
Notifications
You must be signed in to change notification settings - Fork 41
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
Heads up: pathlib.VirtualPath may be coming soon #140
Comments
Thanks @barneygale for letting us know Instead of basing our Accessor layer in Accessor class (the old design that was removed from the pathlib few versions ago) basing S3Path from Path or from _PathBase it's not really matter from our point of view. The issue that we do have is with how other packages react to path objects. for example Pandas # for local path
pandas.read_csv('/path/to/table.csv', ...)
# or if you want to work with uri representation
pandas.read_csv('file://localhost/path/to/table.csv', ...) And when you want S3 you can do like this: # for S3 uri representation
pandas.read_csv('s3://path/to/table.csv', ...) Now when you are working with Pandas and Path objects.
from pathlib import Path
from s3path import S3Path
local_path = Path('/path/to/table.csv')
pandas.read_csv(local_path, ...) # it's the same like passing '/path/to/table.csv'
# but in S3Path we braike
s3_path = S3Path('/path/to/table.csv')
pandas.read_csv(s3_path, ...) # it's the same like passing '/path/to/table.csv' instead of s3://path/to/table.csv
from pathlib import Path
local_path = Path('/path/to/table.csv')
pandas.read_csv(local_path.as_uri(), ...) # This will failed. file:///path/to/table.csv is not what Pandas expect
# not the same like:
pandas.read_csv('file://localhost/path/to/table.csv', ...) # This will failed. file:///path/to/table.csv is not what Pandas expect My point is not that Pandas have issues, Pandas was just one example. I think that we still have to think about the general purpose and design of methods like make sense? |
Thanks for the feedback, that makes perfect sense. My hope is that APIs like def read_csv(path: pathlib.PathBase | os.PathLike):
if not isinstance(path, pathlib.PathBase):
path = pathlib.Path(path)
with path.open() as f:
... It will take a while for third-party packages to begin accepting Does that make sense? On On URIs: the implementation of |
This comment was marked as resolved.
This comment was marked as resolved.
Make perfect sense @barneygale Regarding |
Shit sorry, horrible typo on my part. Will correct. |
It'll become a public class, hopefully in time for Python 3.13 (next year) |
Hello - I've published CPython's private If the PyPI package succeeds and matures, I'm hopeful we can make |
@barneygale, I was just reading the Python 3.13 release notes. Congrats on the inclusion of your |
@barneygale Why |
Both
|
In my logic all IO oriented factuality need to be in the Path level How ever I full understand the logic you specify :-) |
I've logged a CPython PR that adds a private
pathlib._VirtualPath
class:python/cpython#106337
If/when that lands, it's not much more work to drop the underscore and introduce
pathlib.VirtualPath
to the world. This would be Python 3.13 at the earliest.Would it be suitable for use in s3path? It would be great to hear your feedback!
The text was updated successfully, but these errors were encountered: