Skip to content

Commit 9c8de74

Browse files
d-v-bclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 029c376 commit 9c8de74

3 files changed

Lines changed: 40 additions & 14 deletions

File tree

changes/TBD.bugfix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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.

src/zarr/storage/_fsspec.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
)
1717
from zarr.core.buffer import Buffer
1818
from zarr.errors import ZarrUserWarning
19-
from zarr.storage._utils import _join_paths, normalize_path
19+
from zarr.storage._utils import _join_paths
2020

2121
if TYPE_CHECKING:
2222
from collections.abc import AsyncIterator, Iterable
@@ -127,7 +127,7 @@ def __init__(
127127
) -> None:
128128
super().__init__(read_only=read_only)
129129
self.fs = fs
130-
self.path = normalize_path(path)
130+
self.path = path.rstrip("/")
131131
self.allowed_exceptions = allowed_exceptions
132132

133133
if not self.fs.async_impl:

tests/test_store/test_fsspec.py

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
from zarr.errors import ZarrUserWarning
1818
from zarr.storage import FsspecStore
1919
from zarr.storage._fsspec import _make_async
20-
from zarr.storage._utils import normalize_path
2120
from zarr.testing.store import StoreTests
2221

2322
if TYPE_CHECKING:
@@ -291,21 +290,47 @@ def array_roundtrip(store: FsspecStore) -> None:
291290
parse_version(fsspec.__version__) < parse_version("2024.12.0"),
292291
reason="No AsyncFileSystemWrapper",
293292
)
294-
@pytest.mark.parametrize("path", ["", "/", "//", "foo", "foo/", "/foo", "/foo/", "//foo//"])
295-
def test_fsspec_store_path_normalization(path: str) -> None:
296-
"""`FsspecStore.path` is normalized to the canonical form, matching
297-
`normalize_path`, regardless of the surface representation the caller
298-
supplies.
299-
300-
Regression test for https://github.com/zarr-developers/zarr-python/issues/3922
301-
-- when a caller passed `path="/"` the leading slash flowed through
302-
unmodified to subsequent `_join_paths([self.path, key])` calls, producing
303-
`"//key"` and missing the underlying object.
293+
@pytest.mark.parametrize(
294+
("path", "expected"),
295+
[
296+
("", ""),
297+
("/", ""),
298+
("//", ""),
299+
("foo", "foo"),
300+
("foo/", "foo"),
301+
# Leading slashes are preserved: `path` is an opaque string passed to the
302+
# underlying fsspec backend, and for some backends (notably LocalFileSystem)
303+
# a leading slash is semantically meaningful. The store only strips trailing
304+
# slashes so that `_join_paths([self.path, key])` produces a single separator
305+
# between the root and the key.
306+
("/foo", "/foo"),
307+
("/foo/", "/foo"),
308+
("//foo//", "//foo"),
309+
# An absolute filesystem path round-trips intact.
310+
("/home/runner/data.zarr", "/home/runner/data.zarr"),
311+
("/home/runner/data.zarr/", "/home/runner/data.zarr"),
312+
],
313+
)
314+
def test_fsspec_store_path_normalization(path: str, expected: str) -> None:
315+
"""`FsspecStore.path` strips trailing slashes only.
316+
317+
Regression test for two related bugs:
318+
319+
- https://github.com/zarr-developers/zarr-python/issues/3922 -- a caller
320+
passing ``path="/"`` resulted in `_join_paths(["/", "key"])` producing
321+
``"//key"``, which missed the underlying object on backends like
322+
``ReferenceFileSystem``. ``"/"`` must collapse to ``""``.
323+
324+
- Titiler regression after #3924 -- normalizing ``path`` via
325+
``normalize_path`` corrupted absolute filesystem paths by stripping the
326+
leading slash, turning ``/home/runner/data.zarr`` into
327+
``home/runner/data.zarr`` (a relative path that doesn't exist). Leading
328+
slashes must be preserved.
304329
"""
305330
sync_fs = fsspec.filesystem("memory")
306331
fs = _make_async(sync_fs)
307332
store = FsspecStore(fs=fs, path=path)
308-
assert store.path == normalize_path(path)
333+
assert store.path == expected
309334

310335

311336
@pytest.mark.skipif(

0 commit comments

Comments
 (0)