disallow / keys

Author: flying-sheep

src/anndata/_io/specs/registry.py

@@ -364,41 +364,47 @@ def write_elem(
 
         from anndata._io.zarr import is_group_consolidated
 
-        # we allow stores to have a prefix like /uns which are then written to with keys like /uns/foo
-        if k.startswith(store.name) and k != "/":
-            k = str(PurePosixPath(k).relative_to(store.name))
-
-        # Apart from this code, we also ban keys containing slashes in `write_adata`/`write_h5ad`
-        # for AnnData elements other than `obs`, `var`, and `uns`.
-        if "/" in k and k != "/":
-            if isinstance(store, ZarrGroup) or settings.disallow_forward_slash_in_h5ad:
-                msg = f"Forward slashes are not allowed in keys in {type(store)}"
-                raise ValueError(msg)
-            msg = "Forward slashes will be written differently in a future anndata version"
-            warn(msg, FutureWarning)
-
         if isinstance(store, h5py.File):
             store = store["/"]
-
-        dest_type = type(store)
-
-        if is_group_consolidated(store, strict=False):
+        elif is_group_consolidated(store, strict=False):
             msg = "Cannot overwrite/edit a store with consolidated metadata"
             raise ValueError(msg)
+
         if k == "/":
+            if store.name != "/":
+                msg = f"'/' is not in the subpath of {store.name!r}"
+                raise ValueError(msg)
+
             if isinstance(store, ZarrGroup):
                 from zarr.core.sync import sync
 
                 sync(store.store.clear())
             else:
                 store.clear()
-        elif k in store:
-            del store[k]
+        else:
+            # we allow stores to have a prefix like /uns which are then written to with keys like /uns/foo
+            if k.startswith("/"):
+                k = str(PurePosixPath(k).relative_to(store.name, walk_up=False))
+
+            # Apart from this code, we also ban keys containing slashes in `write_adata`/`write_h5ad`
+            # for AnnData elements other than `obs`, `var`, and `uns`.
+            if "/" in k:
+                if (
+                    isinstance(store, ZarrGroup)
+                    or settings.disallow_forward_slash_in_h5ad
+                ):
+                    msg = f"Forward slashes are not allowed in keys in {type(store)}"
+                    raise ValueError(msg)
+                msg = "Forward slashes will be written differently in a future anndata version"
+                warn(msg, FutureWarning)
+
+            if k in store:
+                del store[k]
 
         # Normalize array-API (e.g., JAX/CuPy) even if not AnnData
         elem = normalize_nested(elem)
 
-        write_func = self.find_write_func(dest_type, elem, modifiers)
+        write_func = self.find_write_func(type(store), elem, modifiers)
 
         if self.callback is None:
             return write_func(store, k, elem, dataset_kwargs=dataset_kwargs)

tests/test_io_elementwise.py

@@ -690,9 +690,7 @@ def test_categorical_order_type(store):
 
 
 def test_override_specification():
-    """
-    Test that trying to overwrite an existing encoding raises an error.
-    """
+    """Test that trying to overwrite an existing encoding raises an error."""
     from copy import deepcopy
 
     registry = deepcopy(_REGISTRY)

tests/test_readwrite.py

@@ -19,6 +19,7 @@
 import anndata as ad
 from anndata._io.specs.registry import IORegistryError
 from anndata._io.zarr import open_write_group
+from anndata._types import AnnDataElem
 from anndata.compat import (
     CSArray,
     CSMatrix,
@@ -35,6 +36,7 @@
     jnp,
     jnp_array_or_idempotent,
 )
+from anndata.utils import get_literal_members
 
 if TYPE_CHECKING:
     from collections.abc import Callable, Generator
@@ -960,20 +962,20 @@ def test_h5py_attr_limit(tmp_path):
 
 
 @pytest.mark.parametrize(
-    "elem_key", ["obs", "var", "obsm", "varm", "layers", "obsp", "varp", "uns"]
+    "elem_key", set(get_literal_members(AnnDataElem)) - {"raw", "X"}
 )
 @pytest.mark.parametrize("store_type", ["zarr", "h5ad"])
 @pytest.mark.parametrize(
     "disallow_forward_slash_in_h5ad", [True, False], ids=["ban_slash", "allow_slash"]
 )
 def test_forward_slash_key(
-    elem_key: Literal["obs", "var", "obsm", "varm", "layers", "obsp", "varp", "uns"],
+    *,
     tmp_path: Path,
+    elem_key: AnnDataElem,
     store_type: Literal["zarr", "h5ad"],
-    *,
     disallow_forward_slash_in_h5ad: bool,
-):
-    a = ad.AnnData(np.ones((10, 10)))
+) -> None:
+    a = ad.AnnData(shape=(10, 10))
     getattr(a, elem_key)["bad/key"] = np.ones(
         (10,) if elem_key in ["obs", "var"] else (10, 10)
     )
@@ -1008,6 +1010,36 @@ def test_forward_slash_key(
         assert not elem.keys()
 
 
+@pytest.mark.parametrize(
+    "elem_key", set(get_literal_members(AnnDataElem)) - {"raw", "X"}
+)
+@pytest.mark.parametrize("store_type", ["zarr", "h5ad"])
+@pytest.mark.parametrize("key", ["/", "/y"])
+def test_leading_slash_error(
+    *,
+    tmp_path: Path,
+    elem_key: AnnDataElem,
+    store_type: Literal["zarr", "h5ad"],
+    key: str,
+) -> None:
+    a = ad.AnnData(shape=(10, 10))
+    getattr(a, elem_key)[key] = np.ones(
+        (10,) if elem_key in ["obs", "var"] else (10, 10)
+    )
+    path = tmp_path / f"test.{store_type}"
+
+    # “not in the subpath” is raised by e.g. `write_elem(g["z"], "/y", ...)`,
+    # while “Forward slashes” is raised earlier by `write_anndata`/`write_h5ad`
+    with pytest.raises(ValueError, match=r"not in the subpath|Forward slashes"):
+        getattr(a, f"write_{store_type}")(path)
+
+    elem = getattr(getattr(ad, f"read_{store_type}")(path), elem_key)
+    if elem_key in {"obs", "var"}:
+        assert set(elem.columns) == {"_index"}
+    else:
+        assert not elem.keys()
+
+
 @pytest.mark.skipif(
     bool(find_spec("xarray")),
     reason="Xarray is installed so `read_{elem_}lazy` will not error",