diff --git a/upath/core.py b/upath/core.py index 90ebac60..6e35e208 100644 --- a/upath/core.py +++ b/upath/core.py @@ -1880,7 +1880,7 @@ def unlink(self, missing_ok: bool = False) -> None: return self.fs.rm(self.path, recursive=False) - def rmdir(self, recursive: bool = True) -> None: # fixme: non-standard + def rmdir(self, recursive: bool = UNSET_DEFAULT) -> None: # fixme: non-standard """ Remove this directory. @@ -1890,13 +1890,26 @@ def rmdir(self, recursive: bool = True) -> None: # fixme: non-standard as it supports a `recursive` parameter to remove non-empty directories and defaults to recursive deletion. - This behavior is likely to change in future releases once - `.delete()` is introduced. + Removing a non-empty directory without passing `recursive` + raises a FutureWarning: in universal-pathlib 0.4.0 the default + becomes non-recursive, matching pathlib.Path.rmdir(), and + `.delete()` will remove a directory and its contents. """ if not self.is_dir(): raise NotADirectoryError(str(self)) - if not recursive and next(self.iterdir()): # type: ignore[arg-type] + if recursive is UNSET_DEFAULT: + recursive = True + if next(self.iterdir(), None) is not None: + warnings.warn( + f"{type(self).__name__}.rmdir() currently removes a non-empty" + " directory and its contents. In universal-pathlib 0.4.0 it will" + " raise, like pathlib.Path.rmdir() does. Pass recursive=True to" + " keep removing the contents.", + FutureWarning, + stacklevel=2, + ) + elif not recursive and next(self.iterdir(), None) is not None: raise OSError(f"Not recursive and directory not empty: {self}") self.fs.rm(self.path, recursive=recursive) diff --git a/upath/implementations/cloud.py b/upath/implementations/cloud.py index 0f76b4c1..984fd97c 100644 --- a/upath/implementations/cloud.py +++ b/upath/implementations/cloud.py @@ -216,7 +216,10 @@ def root(self) -> str: def iterdir(self) -> Iterator[Self]: try: yield from super().iterdir() - except NotImplementedError: + except (NotImplementedError, ValueError): + # listing a namespace (e.g. a user's repositories) is unsupported: + # older huggingface_hub raises NotImplementedError, newer versions + # reject the single-segment repository id with a ValueError raise UnsupportedOperation def touch(self, mode: int = 0o666, exist_ok: bool = True) -> None: diff --git a/upath/tests/cases.py b/upath/tests/cases.py index 22587fa9..8597f46d 100644 --- a/upath/tests/cases.py +++ b/upath/tests/cases.py @@ -962,6 +962,19 @@ def test_rmdir_not_empty(self): with pytest.raises(OSError, match="not empty"): p.rmdir(recursive=False) + def test_rmdir_not_empty_warns_about_the_future_default(self): + p = self.path.joinpath("folder1") + with pytest.warns(FutureWarning, match="0.4.0"): + p.rmdir() + assert not p.exists() + + def test_rmdir_not_empty_recursive_does_not_warn(self): + p = self.path.joinpath("folder1") + with warnings.catch_warnings(): + warnings.simplefilter("error", FutureWarning) + p.rmdir(recursive=True) + assert not p.exists() + def test_fsspec_compat(self): fs = self.path.fs content = b"a,b,c\n1,2,3\n4,5,6" diff --git a/upath/tests/test_extensions.py b/upath/tests/test_extensions.py index f29ba999..fafab0c0 100644 --- a/upath/tests/test_extensions.py +++ b/upath/tests/test_extensions.py @@ -79,6 +79,15 @@ def test_is_not_wrapped_class(self): def test_chmod(self): self.path.joinpath("file1.txt").chmod(777) + @overrides_base + def test_rmdir_not_empty_warns_about_the_future_default(self): + # wrapping a pathlib.Path, whose rmdir() already raises on a + # non-empty directory: there is no future change to warn about + p = self.path.joinpath("folder1") + with pytest.raises(OSError): + p.rmdir() + assert p.exists() + @overrides_base @pytest.mark.skipif( sys.version_info < (3, 12), reason="storage options only handled in 3.12+"