Skip to content
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

ENH: Allow .resolve() with CloudPaths #151

Closed
remi-braun opened this issue Jul 5, 2021 · 3 comments · Fixed by #302
Closed

ENH: Allow .resolve() with CloudPaths #151

remi-braun opened this issue Jul 5, 2021 · 3 comments · Fixed by #302
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@remi-braun
Copy link

Hello,

In order to simplify the code and unify the API, would it be possible to allow the .resolve() function to CloudPaths, even if this function does nothing ?

To go from:

path = AnyPath(path)
if not isinstance(path, CloudPath):
   path = path.resolve()

to

path = AnyPath(path).resolve()

Thanks a lot

@pjbull
Copy link
Member

pjbull commented Jul 5, 2021

We already have a _resolve method that we use to make sure there are no relative paths in the key part of the CloudPath. Seems reasonable to expose this functionality in the .resolve method:

# The function resolve is not available on Pure paths because it removes relative
# paths and symlinks. We _just_ want the relative path resolution for
# cloud paths, so the other logic is removed. Also, we can assume that
# cloud paths are absolute.
#
# Based on resolve from pathlib:
# https://github.com/python/cpython/blob/3.8/Lib/pathlib.py#L316-L359
def _resolve(path: PurePosixPath) -> str:
sep = "/"
# rebuild path from parts
newpath = ""
for name in str(path).split(sep):
if not name or name == ".":
# current dir, nothing to add
continue
if name == "..":
# parent dir, drop right-most part
newpath, _, _ = newpath.rpartition(sep)
continue
newpath = newpath + sep + name
return newpath or sep

The tricky part will be to get all the test cases included for this kind of functionality. (E.g., path that ends in .., .. that goes above the bucket level, etc.)

@pjbull pjbull added enhancement New feature or request good first issue Good for newcomers labels Jul 5, 2021
@remi-braun
Copy link
Author

For sure these moot points can be tricky, however do they really happen when handling cloud paths ? (especially since the relative_to function is not implemented)
Maybe just add a warning in the documentation first, and deal with this if the users are asking 😛

@pjbull
Copy link
Member

pjbull commented Jun 1, 2022

Added as a no-op for API consistency in #230

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants