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

Have relative_to return PosixUPath #215

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ python_requires = >=3.8
zip_safe = False
packages = find:
install_requires=
fsspec>=2022.1.0
fsspec >=2022.1.0,!=2024.3.1

[options.extras_require]
tests =
Expand Down
31 changes: 3 additions & 28 deletions upath/_compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,34 +253,9 @@ def with_suffix(self, suffix):
self.drive, self.root, self._tail[:-1] + [name]
)

def relative_to(self, other, /, *_deprecated, walk_up=False):
if _deprecated:
msg = (
"support for supplying more than one positional argument "
"to pathlib.PurePath.relative_to() is deprecated and "
"scheduled for removal in Python 3.14"
)
warnings.warn(
f"pathlib.PurePath.relative_to(*args) {msg}",
DeprecationWarning,
stacklevel=2,
)
other = self.with_segments(other, *_deprecated)
for step, path in enumerate([other] + list(other.parents)): # noqa: B007
if self.is_relative_to(path):
break
elif not walk_up:
raise ValueError(
f"{str(self)!r} is not in the subpath of {str(other)!r}"
)
elif path.name == "..":
raise ValueError(f"'..' segment in {str(other)!r} cannot be walked")
else:
raise ValueError(
f"{str(self)!r} and {str(other)!r} have different anchors"
)
parts = [".."] * step + self._tail[len(path._tail) :]
return self.with_segments(*parts)
# NOTE relative_to was elevated to UPath as otherwise this would
# cause a circular dependency
# def relative_to(self, other, /, *_deprecated, walk_up=False):

def is_relative_to(self, other, /, *_deprecated):
if _deprecated:
Expand Down
31 changes: 29 additions & 2 deletions upath/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,14 +666,41 @@ def __hash__(self):
can add `fsspec.utils.tokenize(storage_options)`
"""
return hash((self.protocol, self.path))

def relative_to(self, other, /, *_deprecated, walk_up=False):
if isinstance(other, UPath) and self.storage_options != other.storage_options:
raise ValueError(
"paths have different storage_options:"
f" {self.storage_options!r} != {other.storage_options!r}"
)
return super().relative_to(other, *_deprecated, walk_up=walk_up)

if _deprecated:
msg = (
"support for supplying more than one positional argument "
"to pathlib.PurePath.relative_to() is deprecated and "
"scheduled for removal in Python 3.14"
)
warnings.warn(
f"pathlib.PurePath.relative_to(*args) {msg}",
DeprecationWarning,
stacklevel=2,
)
other = self.with_segments(other, *_deprecated)
for step, path in enumerate([other] + list(other.parents)): # noqa: B007
if self.is_relative_to(path):
break
elif not walk_up:
raise ValueError(
f"{str(self)!r} is not in the subpath of {str(other)!r}"
)
elif path.name == "..":
raise ValueError(f"'..' segment in {str(other)!r} cannot be walked")
else:
raise ValueError(
f"{str(self)!r} and {str(other)!r} have different anchors"
)
parts = [".."] * step + self._tail[len(path._tail) :]
return UPath(*parts, **self._storage_options)

def is_relative_to(self, other, /, *_deprecated):
if isinstance(other, UPath) and self.storage_options != other.storage_options:
Expand Down
3 changes: 1 addition & 2 deletions upath/implementations/cloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ def iterdir(self):

def relative_to(self, other, /, *_deprecated, walk_up=False):
# use the parent implementation for the ValueError logic
super().relative_to(other, *_deprecated, walk_up=False)
return self
return super().relative_to(other, *_deprecated, walk_up=walk_up)


class GCSPath(CloudPath):
Expand Down
1 change: 1 addition & 0 deletions upath/implementations/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from typing import Collection
from typing import MutableMapping
from urllib.parse import SplitResult
import warnings

from upath.core import UPath

Expand Down
19 changes: 19 additions & 0 deletions upath/tests/implementations/test_azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from upath import UPath
from upath.implementations.cloud import AzurePath
from upath.implementations.local import PosixUPath

from ..cases import BaseTests
from ..utils import skip_on_windows
Expand Down Expand Up @@ -61,3 +62,21 @@ def test_broken_mkdir(self):

(path / "file").write_text("foo")
assert path.exists()

def test_relative_to(self):
rel_path = UPath("az:///test_bucket/file.txt").relative_to(UPath("az:///test_bucket"))
assert isinstance(rel_path, PosixUPath)
assert not rel_path.is_absolute()
assert 'file.txt' == rel_path.path

walk_path = UPath("az:///test_bucket/file.txt").relative_to(UPath("az:///other_test_bucket"), walk_up=True)
assert isinstance(walk_path, PosixUPath)
assert not walk_path.is_absolute()
assert '../test_bucket/file.txt' == walk_path.path

with pytest.raises(ValueError):
UPath("az:///test_bucket/file.txt").relative_to(UPath("az:///prod_bucket"))

with pytest.raises(ValueError):
UPath("az:///test_bucket/file.txt").relative_to(UPath("file:///test_bucket"))

20 changes: 19 additions & 1 deletion upath/tests/implementations/test_local.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest

from upath import UPath
from upath.implementations.local import LocalPath
from upath.implementations.local import LocalPath, PosixUPath
from upath.tests.cases import BaseTests
from upath.tests.utils import skip_on_windows
from upath.tests.utils import xfail_if_version
Expand All @@ -17,6 +17,24 @@ def path(self, local_testdir):
def test_is_LocalPath(self):
assert isinstance(self.path, LocalPath)

def test_relative_to(self):
rel_path = UPath("file:///test_bucket/file.txt").relative_to(UPath("file:///test_bucket"))
assert isinstance(rel_path, PosixUPath)
assert not rel_path.is_absolute()
assert 'file.txt' == rel_path.path

walk_path = UPath("file:///test_bucket/file.txt").relative_to(UPath("file:///other_test_bucket"), walk_up=True)
assert isinstance(walk_path, PosixUPath)
assert not walk_path.is_absolute()
assert '../test_bucket/file.txt' == walk_path.path

with pytest.raises(ValueError):
UPath("file:///test_bucket/file.txt").relative_to(UPath("file:///prod_bucket"))

with pytest.raises(ValueError):
UPath("file:///test_bucket/file.txt").relative_to(UPath("s3:///test_bucket"))


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if you could add a test that checks if a ValueError is raised if a UPath of a different protocol is provided as argument.

And a test that checks the cases for the walk_up keyword argument.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a test for s3, not sure what walk_up allows will need more time to look at that to write a test case

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a test for walk_up seems like it's broken for azure

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seemingly its because UPath("az:///other_test_bucket").parents is an empty list which I don't understand


@skip_on_windows
@xfail_if_version("fsspec", lt="2023.10.0", reason="requires fsspec>=2023.10.0")
Expand Down
21 changes: 17 additions & 4 deletions upath/tests/implementations/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
import fsspec
import pytest # noqa: F401

from upath import UPath
from upath.core import UPath
from upath.implementations.local import PosixUPath
from upath.implementations.cloud import S3Path

from ..cases import BaseTests
Expand Down Expand Up @@ -37,9 +38,21 @@ def test_rmdir(self):
self.path.joinpath("file1.txt").rmdir()

def test_relative_to(self):
assert "s3://test_bucket/file.txt" == str(
UPath("s3://test_bucket/file.txt").relative_to(UPath("s3://test_bucket"))
)
rel_path = UPath("s3:///test_bucket/file.txt").relative_to(UPath("s3:///test_bucket"))
assert isinstance(rel_path, PosixUPath)
assert not rel_path.is_absolute()
assert 'file.txt' == rel_path.path

walk_path = UPath("s3:///test_bucket/file.txt").relative_to(UPath("s3:///other_test_bucket"), walk_up=True)
assert isinstance(walk_path, PosixUPath)
assert not walk_path.is_absolute()
assert '../test_bucket/file.txt' == walk_path.path

with pytest.raises(ValueError):
UPath("s3:///test_bucket/file.txt").relative_to(UPath("s3:///prod_bucket"))

with pytest.raises(ValueError):
UPath("s3:///test_bucket/file.txt").relative_to(UPath("file:///test_bucket"))

def test_iterdir_root(self):
client_kwargs = self.path._kwargs["client_kwargs"]
Expand Down