Merge branch 'main' into zipstore-from-path

Author: aulemahal

.pre-commit-config.yaml

@@ -6,7 +6,7 @@ ci:
 default_stages: [pre-commit, pre-push]
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.9.4
+    rev: v0.9.9
     hooks:
     - id: ruff
       args: ["--fix", "--show-fixes"]
@@ -22,7 +22,7 @@ repos:
     - id: check-yaml
     - id: trailing-whitespace
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.14.1
+    rev: v1.15.0
     hooks:
       - id: mypy
         files: src|tests

changes/2850.fix.rst

@@ -0,0 +1,2 @@
+Fixed a bug where ``StorePath`` creation would not apply standard path normalization to the ``path`` parameter,
+which led to the creation of arrays and groups with invalid keys.

docs/user-guide/v3_migration.rst

@@ -124,6 +124,30 @@ The Store class
 The Store API has changed significant in Zarr-Python 3. The most notable changes to the
 Store API are:
 
+Store Import Paths
+^^^^^^^^^^^^^^^^^^
+Several store implementations have moved from the top-level module to ``zarr.storage``:
+
+.. code-block:: diff
+   :caption: Store import changes from v2 to v3
+
+   # Before (v2)
+   - from zarr import MemoryStore, DirectoryStore
+   + from zarr.storage import MemoryStore, LocalStore  # LocalStore replaces DirectoryStore
+
+Common replacements:
+
++-------------------------+------------------------------------+
+| v2 Import               | v3 Import                          |
++=========================+====================================+
+| ``zarr.MemoryStore``    | ``zarr.storage.MemoryStore``       |
++-------------------------+------------------------------------+
+| ``zarr.DirectoryStore`` | ``zarr.storage.LocalStore``        |
++-------------------------+------------------------------------+
+| ``zarr.TempStore``      | Use ``tempfile.TemporaryDirectory``|
+|                         | with ``LocalStore``                |
++-------------------------+------------------------------------+
+
 1. Replaced the ``MutableMapping`` base class in favor of a custom abstract base class
    (:class:`zarr.abc.store.Store`).
 2. Switched to an asynchronous interface for all store methods that result in IO. This

src/zarr/storage/_common.py

@@ -42,7 +42,7 @@ class StorePath:
 
     def __init__(self, store: Store, path: str = "") -> None:
         self.store = store
-        self.path = path
+        self.path = normalize_path(path)
 
     @property
     def read_only(self) -> bool:

src/zarr/testing/strategies.py

@@ -19,6 +19,7 @@
 from zarr.core.sync import sync
 from zarr.storage import MemoryStore, StoreLike
 from zarr.storage._common import _dereference_path
+from zarr.storage._utils import normalize_path
 
 # Copied from Xarray
 _attr_keys = st.text(st.characters(), min_size=1)
@@ -277,11 +278,12 @@ def arrays(
     if a.metadata.zarr_format == 3:
         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 isinstance(root[array_path], Array)
     assert nparray.shape == a.shape
     assert chunk_shape == a.chunks
     assert shard_shape == a.shards
-    assert array_path == a.path, (path, name, array_path, a.name, a.path)
     assert a.basename == name, (a.basename, name)
     assert dict(a.attrs) == expected_attrs
 

tests/test_api.py

@@ -209,7 +209,7 @@ def test_save(store: Store, n_args: int, n_kwargs: int) -> None:
         assert isinstance(array, Array)
         assert_array_equal(array[:], data)
     else:
-        save(store, *args, **kwargs)  # type: ignore[arg-type]
+        save(store, *args, **kwargs)  # type: ignore [arg-type]
         group = open(store)
         assert isinstance(group, Group)
         for array in group.array_values():
@@ -1095,25 +1095,31 @@ async def test_open_falls_back_to_open_group_async() -> None:
     assert group.attrs == {"key": "value"}
 
 
-def test_open_mode_write_creates_group(tmp_path: pathlib.Path) -> None:
+@pytest.mark.parametrize("mode", ["r", "r+", "w", "a"])
+def test_open_modes_creates_group(tmp_path: pathlib.Path, mode: str) -> None:
     # https://github.com/zarr-developers/zarr-python/issues/2490
-    zarr_dir = tmp_path / "test.zarr"
-    group = zarr.open(zarr_dir, mode="w")
-    assert isinstance(group, Group)
+    zarr_dir = tmp_path / f"mode-{mode}-test.zarr"
+    if mode in ["r", "r+"]:
+        # Expect FileNotFoundError to be raised if 'r' or 'r+' mode
+        with pytest.raises(FileNotFoundError):
+            zarr.open(store=zarr_dir, mode=mode)
+    else:
+        group = zarr.open(store=zarr_dir, mode=mode)
+        assert isinstance(group, Group)
 
 
 async def test_metadata_validation_error() -> None:
     with pytest.raises(
         MetadataValidationError,
         match="Invalid value for 'zarr_format'. Expected '2, 3, or None'. Got '3.0'.",
     ):
-        await zarr.api.asynchronous.open_group(zarr_format="3.0")  # type: ignore[arg-type]
+        await zarr.api.asynchronous.open_group(zarr_format="3.0")  # type: ignore [arg-type]
 
     with pytest.raises(
         MetadataValidationError,
         match="Invalid value for 'zarr_format'. Expected '2, 3, or None'. Got '3.0'.",
     ):
-        await zarr.api.asynchronous.open_array(shape=(1,), zarr_format="3.0")  # type: ignore[arg-type]
+        await zarr.api.asynchronous.open_array(shape=(1,), zarr_format="3.0")  # type: ignore [arg-type]
 
 
 @pytest.mark.parametrize(

tests/test_group.py

@@ -130,24 +130,27 @@ async def test_create_creates_parents(store: Store, zarr_format: ZarrFormat) ->
             assert g.attrs == {}
 
 
-def test_group_name_properties(store: Store, zarr_format: ZarrFormat) -> None:
+@pytest.mark.parametrize("store", ["memory"], indirect=True)
+@pytest.mark.parametrize("root_name", ["", "/", "a", "/a"])
+@pytest.mark.parametrize("branch_name", ["foo", "/foo", "foo/bar", "/foo/bar"])
+def test_group_name_properties(
+    store: Store, zarr_format: ZarrFormat, root_name: str, branch_name: str
+) -> None:
     """
-    Test basic properties of groups
+    Test that the path, name, and basename attributes of a group and its subgroups are consistent
     """
-    root = Group.from_store(store=store, zarr_format=zarr_format)
-    assert root.path == ""
-    assert root.name == "/"
-    assert root.basename == ""
+    root = Group.from_store(store=StorePath(store=store, path=root_name), zarr_format=zarr_format)
+    assert root.path == normalize_path(root_name)
+    assert root.name == "/" + root.path
+    assert root.basename == root.path
 
-    foo = root.create_group("foo")
-    assert foo.path == "foo"
-    assert foo.name == "/foo"
-    assert foo.basename == "foo"
-
-    bar = root.create_group("foo/bar")
-    assert bar.path == "foo/bar"
-    assert bar.name == "/foo/bar"
-    assert bar.basename == "bar"
+    branch = root.create_group(branch_name)
+    if root.path == "":
+        assert branch.path == normalize_path(branch_name)
+    else:
+        assert branch.path == "/".join([root.path, normalize_path(branch_name)])
+    assert branch.name == "/" + branch.path
+    assert branch.basename == branch_name.split("/")[-1]
 
 
 @pytest.mark.parametrize("consolidated_metadata", [True, False])
@@ -623,11 +626,13 @@ async def test_group_update_attributes_async(store: Store, zarr_format: ZarrForm
 
 
 @pytest.mark.parametrize("method", ["create_array", "array"])
+@pytest.mark.parametrize("name", ["a", "/a"])
 def test_group_create_array(
     store: Store,
     zarr_format: ZarrFormat,
     overwrite: bool,
     method: Literal["create_array", "array"],
+    name: str,
 ) -> None:
     """
     Test `Group.from_store`
@@ -638,23 +643,26 @@ def test_group_create_array(
     data = np.arange(np.prod(shape)).reshape(shape).astype(dtype)
 
     if method == "create_array":
-        array = group.create_array(name="array", shape=shape, dtype=dtype)
+        array = group.create_array(name=name, shape=shape, dtype=dtype)
         array[:] = data
     elif method == "array":
         with pytest.warns(DeprecationWarning):
-            array = group.array(name="array", data=data, shape=shape, dtype=dtype)
+            array = group.array(name=name, data=data, shape=shape, dtype=dtype)
     else:
         raise AssertionError
 
     if not overwrite:
         if method == "create_array":
             with pytest.raises(ContainsArrayError):
-                a = group.create_array(name="array", shape=shape, dtype=dtype)
+                a = group.create_array(name=name, shape=shape, dtype=dtype)
                 a[:] = data
         elif method == "array":
             with pytest.raises(ContainsArrayError), pytest.warns(DeprecationWarning):
-                a = group.array(name="array", shape=shape, dtype=dtype)
+                a = group.array(name=name, shape=shape, dtype=dtype)
                 a[:] = data
+
+    assert array.path == normalize_path(name)
+    assert array.name == "/" + array.path
     assert array.shape == shape
     assert array.dtype == np.dtype(dtype)
     assert np.array_equal(array[:], data)
@@ -945,20 +953,23 @@ async def test_asyncgroup_delitem(store: Store, zarr_format: ZarrFormat) -> None
         raise AssertionError
 
 
+@pytest.mark.parametrize("name", ["a", "/a"])
 async def test_asyncgroup_create_group(
     store: Store,
+    name: str,
     zarr_format: ZarrFormat,
 ) -> None:
     agroup = await AsyncGroup.from_store(store=store, zarr_format=zarr_format)
-    sub_node_path = "sub_group"
     attributes = {"foo": 999}
-    subnode = await agroup.create_group(name=sub_node_path, attributes=attributes)
-
-    assert isinstance(subnode, AsyncGroup)
-    assert subnode.attrs == attributes
-    assert subnode.store_path.path == sub_node_path
-    assert subnode.store_path.store == store
-    assert subnode.metadata.zarr_format == zarr_format
+    subgroup = await agroup.create_group(name=name, attributes=attributes)
+
+    assert isinstance(subgroup, AsyncGroup)
+    assert subgroup.path == normalize_path(name)
+    assert subgroup.name == "/" + subgroup.path
+    assert subgroup.attrs == attributes
+    assert subgroup.store_path.path == subgroup.path
+    assert subgroup.store_path.store == store
+    assert subgroup.metadata.zarr_format == zarr_format
 
 
 async def test_asyncgroup_create_array(