From 9c8de74f272c79ecbedb4d840057796b1eea92b3 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Mon, 27 Apr 2026 07:13:23 -0400 Subject: [PATCH 1/2] fix(storage): preserve leading slashes in FsspecStore.path #3924 ran the constructor's `path` argument through `normalize_path`, which is intended for zarr logical keys and strips leading slashes. Applied to a filesystem-side root, this turned absolute paths like /home/foo/data.zarr into the relative home/foo/data.zarr, breaking LocalFileSystem-backed FsspecStore for any caller that passed an absolute path. Downstream impact: titiler-xarray's test-upstream job fails on every dataset_3d.zarr fixture access. The original #3922 issue (path="/" producing "//key" via _join_paths) is still resolved: rstrip("/") collapses "/" to "", so the join filter drops it. Trailing slashes are also still stripped. Updates the existing test_fsspec_store_path_normalization parametrization with the new (correct) expectations and adds two absolute-path cases. Co-Authored-By: Claude Opus 4.7 (1M context) --- changes/TBD.bugfix.md | 1 + src/zarr/storage/_fsspec.py | 4 +-- tests/test_store/test_fsspec.py | 49 +++++++++++++++++++++++++-------- 3 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 changes/TBD.bugfix.md diff --git a/changes/TBD.bugfix.md b/changes/TBD.bugfix.md new file mode 100644 index 0000000000..091ce9637e --- /dev/null +++ b/changes/TBD.bugfix.md @@ -0,0 +1 @@ +Restore preservation of leading slashes in `FsspecStore.path`. The previous fix in `#3924` ran `path` through `normalize_path`, which is intended for zarr keys and strips leading slashes — corrupting absolute filesystem paths (e.g. `/home/foo/data.zarr` became `home/foo/data.zarr`) for backends like `LocalFileSystem`. `FsspecStore` now strips trailing slashes only, which still resolves the original `path="/"` issue without breaking absolute-path callers. diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py index 74e5869a66..04df6c54b9 100644 --- a/src/zarr/storage/_fsspec.py +++ b/src/zarr/storage/_fsspec.py @@ -16,7 +16,7 @@ ) from zarr.core.buffer import Buffer from zarr.errors import ZarrUserWarning -from zarr.storage._utils import _join_paths, normalize_path +from zarr.storage._utils import _join_paths if TYPE_CHECKING: from collections.abc import AsyncIterator, Iterable @@ -127,7 +127,7 @@ def __init__( ) -> None: super().__init__(read_only=read_only) self.fs = fs - self.path = normalize_path(path) + self.path = path.rstrip("/") self.allowed_exceptions = allowed_exceptions if not self.fs.async_impl: diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py index 00b989a5de..2570f4a0ce 100644 --- a/tests/test_store/test_fsspec.py +++ b/tests/test_store/test_fsspec.py @@ -17,7 +17,6 @@ from zarr.errors import ZarrUserWarning from zarr.storage import FsspecStore from zarr.storage._fsspec import _make_async -from zarr.storage._utils import normalize_path from zarr.testing.store import StoreTests if TYPE_CHECKING: @@ -291,21 +290,47 @@ def array_roundtrip(store: FsspecStore) -> None: parse_version(fsspec.__version__) < parse_version("2024.12.0"), reason="No AsyncFileSystemWrapper", ) -@pytest.mark.parametrize("path", ["", "/", "//", "foo", "foo/", "/foo", "/foo/", "//foo//"]) -def test_fsspec_store_path_normalization(path: str) -> None: - """`FsspecStore.path` is normalized to the canonical form, matching - `normalize_path`, regardless of the surface representation the caller - supplies. - - Regression test for https://github.com/zarr-developers/zarr-python/issues/3922 - -- when a caller passed `path="/"` the leading slash flowed through - unmodified to subsequent `_join_paths([self.path, key])` calls, producing - `"//key"` and missing the underlying object. +@pytest.mark.parametrize( + ("path", "expected"), + [ + ("", ""), + ("/", ""), + ("//", ""), + ("foo", "foo"), + ("foo/", "foo"), + # Leading slashes are preserved: `path` is an opaque string passed to the + # underlying fsspec backend, and for some backends (notably LocalFileSystem) + # a leading slash is semantically meaningful. The store only strips trailing + # slashes so that `_join_paths([self.path, key])` produces a single separator + # between the root and the key. + ("/foo", "/foo"), + ("/foo/", "/foo"), + ("//foo//", "//foo"), + # An absolute filesystem path round-trips intact. + ("/home/runner/data.zarr", "/home/runner/data.zarr"), + ("/home/runner/data.zarr/", "/home/runner/data.zarr"), + ], +) +def test_fsspec_store_path_normalization(path: str, expected: str) -> None: + """`FsspecStore.path` strips trailing slashes only. + + Regression test for two related bugs: + + - https://github.com/zarr-developers/zarr-python/issues/3922 -- a caller + passing ``path="/"`` resulted in `_join_paths(["/", "key"])` producing + ``"//key"``, which missed the underlying object on backends like + ``ReferenceFileSystem``. ``"/"`` must collapse to ``""``. + + - Titiler regression after #3924 -- normalizing ``path`` via + ``normalize_path`` corrupted absolute filesystem paths by stripping the + leading slash, turning ``/home/runner/data.zarr`` into + ``home/runner/data.zarr`` (a relative path that doesn't exist). Leading + slashes must be preserved. """ sync_fs = fsspec.filesystem("memory") fs = _make_async(sync_fs) store = FsspecStore(fs=fs, path=path) - assert store.path == normalize_path(path) + assert store.path == expected @pytest.mark.skipif( From 3db5e6a7412d9c6c46d337e45e10194d73aae0f5 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Mon, 27 Apr 2026 07:17:06 -0400 Subject: [PATCH 2/2] docs: changelog name --- changes/{TBD.bugfix.md => 3296.bugfix.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changes/{TBD.bugfix.md => 3296.bugfix.md} (100%) diff --git a/changes/TBD.bugfix.md b/changes/3296.bugfix.md similarity index 100% rename from changes/TBD.bugfix.md rename to changes/3296.bugfix.md