Merge branch 'main' into release-improvements

Author: maxrjones

changes/3924.bugfix.md

@@ -535,7 +535,7 @@ def all_equal(self, other: Any, equal_nan: bool = True) -> bool:
             and self._data.dtype.kind not in ("U", "S", "T", "O", "V")
         ):
             _data, other = np.broadcast_arrays(self._data, np.asarray(other, self._data.dtype))
-            void_dtype = "V" + str(_data.dtype.itemsize)
+            void_dtype = f"V{_data.dtype.itemsize}"
             return np.array_equal(_data.view(void_dtype), other.view(void_dtype))
         # use array_equal to obtain equal_nan=True functionality
         # Since fill-value is a scalar, isn't there a faster path than allocating a new array for fill value

src/zarr/core/buffer/core.py

@@ -840,7 +840,7 @@ def name(self) -> str:
             # follow h5py convention: add leading slash
             name = self.path
             if name[0] != "/":
-                name = "/" + name
+                name = f"/{name}"
             return name
         return "/"
 

src/zarr/core/group.py

@@ -133,7 +133,7 @@ def _reload_config() -> None:
 
 def fully_qualified_name(cls: type) -> str:
     module = cls.__module__
-    return module + "." + cls.__qualname__
+    return f"{module}.{cls.__qualname__}"
 
 
 def register_codec(key: str, codec_cls: type[Codec], *, qualname: str | None = None) -> None:

src/zarr/registry.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 _dereference_path
 
 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
         self.allowed_exceptions = allowed_exceptions
 
         if not self.fs.async_impl:
@@ -282,7 +282,7 @@ async def get(
         # docstring inherited
         if not self._is_open:
             await self._open()
-        path = _join_paths([self.path, key])
+        path = _dereference_path(self.path, key)
 
         try:
             if byte_range is None:
@@ -329,7 +329,7 @@ async def set(
             raise TypeError(
                 f"FsspecStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead."
             )
-        path = _join_paths([self.path, key])
+        path = _dereference_path(self.path, key)
         # write data
         if byte_range:
             raise NotImplementedError
@@ -338,7 +338,7 @@ async def set(
     async def delete(self, key: str) -> None:
         # docstring inherited
         self._check_writable()
-        path = _join_paths([self.path, key])
+        path = _dereference_path(self.path, key)
         try:
             await self.fs._rm(path)
         except FileNotFoundError:
@@ -354,14 +354,14 @@ async def delete_dir(self, prefix: str) -> None:
             )
         self._check_writable()
 
-        path_to_delete = _join_paths([self.path, prefix])
+        path_to_delete = _dereference_path(self.path, prefix)
 
         with suppress(*self.allowed_exceptions):
             await self.fs._rm(path_to_delete, recursive=True)
 
     async def exists(self, key: str) -> bool:
         # docstring inherited
-        path = _join_paths([self.path, key])
+        path = _dereference_path(self.path, key)
         exists: bool = await self.fs._exists(path)
         return exists
 
@@ -378,7 +378,7 @@ async def get_partial_values(
             starts: list[int | None] = []
             stops: list[int | None] = []
             for key, byte_range in key_ranges:
-                paths.append(_join_paths([self.path, key]))
+                paths.append(_dereference_path(self.path, key))
                 if byte_range is None:
                     starts.append(None)
                     stops.append(None)
@@ -408,7 +408,7 @@ async def get_partial_values(
     async def list(self) -> AsyncIterator[str]:
         # docstring inherited
         allfiles = await self.fs._find(self.path, detail=False, withdirs=False)
-        for onefile in (a.removeprefix(self.path + "/") for a in allfiles):
+        for onefile in (a.removeprefix(f"{self.path}/") for a in allfiles):
             yield onefile
 
     async def list_dir(self, prefix: str) -> AsyncIterator[str]:
@@ -418,7 +418,7 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
             allfiles = await self.fs._ls(prefix, detail=False)
         except FileNotFoundError:
             return
-        for onefile in (a.replace(prefix + "/", "") for a in allfiles):
+        for onefile in (a.replace(f"{prefix}/", "") for a in allfiles):
             yield onefile.removeprefix(self.path).removeprefix("/")
 
     async def list_prefix(self, prefix: str) -> AsyncIterator[str]:
@@ -429,7 +429,7 @@ async def list_prefix(self, prefix: str) -> AsyncIterator[str]:
             yield onefile.removeprefix(f"{self.path}/")
 
     async def getsize(self, key: str) -> int:
-        path = _join_paths([self.path, key])
+        path = _dereference_path(self.path, key)
         info = await self.fs._info(path)
 
         size = info.get("size")

src/zarr/storage/_fsspec.py

@@ -217,9 +217,9 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
             # a pseudo directory when there's a nested item and we're listing an
             # intermediate level.
             keys_unique = {
-                key.removeprefix(prefix + "/").split("/")[0]
+                key.removeprefix(f"{prefix}/").split("/")[0]
                 for key in self._store_dict
-                if key.startswith(prefix + "/") and key != prefix
+                if key.startswith(f"{prefix}/") and key != prefix
             }
 
         for key in keys_unique:
@@ -822,7 +822,7 @@ async def delete(self, key: str) -> None:
 
     async def list(self) -> AsyncIterator[str]:
         # docstring inherited
-        prefix = self.path + "/" if self.path else ""
+        prefix = f"{self.path}/" if self.path else ""
         async for key in super().list():
             if key.startswith(prefix):
                 yield key.removeprefix(prefix)
@@ -832,7 +832,7 @@ async def list_prefix(self, prefix: str) -> AsyncIterator[str]:
         # Manual concatenation instead of _join_paths because we need "path/"
         # as the prefix when prefix is empty (to list all keys under self.path)
         full_prefix = f"{self.path}/{prefix}" if self.path else prefix
-        path_prefix = self.path + "/" if self.path else ""
+        path_prefix = f"{self.path}/" if self.path else ""
         async for key in super().list_prefix(full_prefix):
             yield key.removeprefix(path_prefix)
 

src/zarr/storage/_memory.py

@@ -185,6 +185,53 @@ def _join_paths(paths: Iterable[str]) -> str:
     return "/".join(filter(lambda v: v != "", paths))
 
 
+def _dereference_path(root: str, path: str) -> str:
+    """
+    Combine a store-side root with a key into a single fully-qualified path.
+
+    Unlike `_join_paths`, this is purpose-built for the case where `root` is
+    an opaque backend-side prefix that may use `"/"` as a sentinel for "root
+    of the filesystem" (notably for fsspec's `ReferenceFileSystem`). A
+    trailing `"/"` is stripped from `root` before joining; if `root` is then
+    empty, the bare `path` is returned so that joining `"/"` with `"key"`
+    yields `"key"` rather than `"//key"`. A trailing `"/"` on the result is
+    also stripped.
+
+    Leading slashes on `root` are preserved -- a backend-side path like
+    `"/home/foo/data.zarr"` is an absolute filesystem path for
+    `LocalFileSystem` and must not lose its leading separator.
+
+    Parameters
+    ----------
+    root : str
+        The backend-side root of a store. May be `""`, `"/"`, an absolute
+        filesystem path, or a backend-specific prefix.
+    path : str
+        The key within the store, typically a zarr key like `"zarr.json"`
+        or `"a/b/c/zarr.json"`.
+
+    Returns
+    -------
+    str
+        `root` and `path` joined by a single `"/"`, with the `"/"` sentinel
+        collapsed and trailing slashes removed.
+
+    Examples
+    --------
+    ```python
+    from zarr.storage._utils import _dereference_path
+    _dereference_path("/", "zarr.json")          # 'zarr.json'
+    _dereference_path("", "zarr.json")           # 'zarr.json'
+    _dereference_path("/home/foo", "zarr.json")  # '/home/foo/zarr.json'
+    _dereference_path("/home/foo/", "zarr.json") # '/home/foo/zarr.json'
+    _dereference_path("bucket/p", "zarr.json")   # 'bucket/p/zarr.json'
+    ```
+    """
+    root = root.rstrip("/")
+    path = f"{root}/{path}" if root else path
+    return path.rstrip("/")
+
+
 def _relativize_path(*, path: str, prefix: str) -> str:
     """
     Make a "/"-delimited path relative to some prefix. If the prefix is '', then the path is
@@ -220,10 +267,10 @@ def _relativize_path(*, path: str, prefix: str) -> str:
     if prefix == "":
         return path
     else:
-        _prefix = prefix + "/"
+        _prefix = f"{prefix}/"
         if not path.startswith(_prefix):
             raise ValueError(f"The first component of {path} does not start with {prefix}.")
-        return path.removeprefix(f"{prefix}/")
+        return path.removeprefix(_prefix)
 
 
 def _normalize_paths(paths: Iterable[str]) -> tuple[str, ...]:

src/zarr/storage/_utils.py

@@ -285,8 +285,8 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]:
                     yield key
         else:
             for key in keys:
-                if key.startswith(prefix + "/") and key.strip("/") != prefix:
-                    k = key.removeprefix(prefix + "/").split("/")[0]
+                if key.startswith(f"{prefix}/") and key.strip("/") != prefix:
+                    k = key.removeprefix(f"{prefix}/").split("/")[0]
                     if k not in seen:
                         seen.add(k)
                         yield k

src/zarr/storage/_zip.py

@@ -449,8 +449,8 @@ async def test_list(self, store: S) -> None:
         prefix = "foo"
         data = self.buffer_cls.from_bytes(b"")
         store_dict = {
-            prefix + "/zarr.json": data,
-            **{prefix + f"/c/{idx}": data for idx in range(10)},
+            f"{prefix}/zarr.json": data,
+            **{f"{prefix}/c/{idx}": data for idx in range(10)},
         }
         await store._set_many(store_dict.items())
         expected_sorted = sorted(store_dict.keys())
@@ -536,10 +536,10 @@ async def test_list_dir(self, store: S) -> None:
             await store._set_many(store_dict.items())
 
             keys_observed = await _collect_aiterator(store.list_dir(root))
-            keys_expected = {k.removeprefix(root + "/").split("/")[0] for k in store_dict}
+            keys_expected = {k.removeprefix(f"{root}/").split("/")[0] for k in store_dict}
             assert sorted(keys_observed) == sorted(keys_expected)
 
-            keys_observed = await _collect_aiterator(store.list_dir(root + "/"))
+            keys_observed = await _collect_aiterator(store.list_dir(f"{root}/"))
             assert sorted(keys_expected) == sorted(keys_observed)
 
     async def test_set_if_not_exists(self, store: S) -> None:

src/zarr/testing/store.py

@@ -323,7 +323,7 @@ def arrays(
         assert a.fill_value is not None
     assert a.name is not None
     assert a.path == normalize_path(array_path)
-    assert a.name == "/" + a.path
+    assert a.name == f"/{a.path}"
     assert isinstance(root[array_path], Array)
     assert nparray.shape == a.shape