Repository: zarr-developers/zarr-python

Issue:
[v3] zarr 3 fails to create child groups
### Zarr version

3.0.0.alpha5

### Numcodecs version

NA

### Python Version

3.11

### Operating System

Mac

### Installation

pip

### Description

The v3 branch is missing some important behavior around the implicit creation of sub-nodes. In 2.x, this behavior existed:

```python
In [1]: import zarr

In [2]: store = {}

In [3]: zarr.create(shape=(10, 10), store=store, path='foo/bar/spam')
Out[3]: <zarr.core.Array '/foo/bar/spam' (10, 10) float64>

In [4]: list(store)
Out[4]: ['.zgroup', 'foo/.zgroup', 'foo/bar/.zgroup', 'foo/bar/spam/.zarray']

In [5]: g = zarr.open_group(store=store)

In [6]: g
Out[6]: <zarr.hierarchy.Group '/'>
```

Note the creation of sub groups `foo`, `foo.bar`, and `foo.bar.spam`.

### Steps to reproduce

```python
In [1]: import zarr

In [2]: store = await zarr.store.MemoryStore.open(mode='w')

In [3]: zarr.create(shape=(10, 10), store=store, path='foo/bar/spam')
Out[3]: <Array memory://4841759424/foo/bar/spam shape=(10, 10) dtype=float64>

In [4]: list(store._store_dict)
Out[4]: ['foo/bar/spam/zarr.json']
```

Note that zarr failed to create `zarr.json` objects for any of the parent groups.

### Additional output

_No response_

Discussion:
jhamman: Thanks @TomAugspurger  -- I'll plan to review this ASAP. 

> To accomplish this semi-safely and efficiently, I've required a new setdefault method on the Store class. The idea is to use an atomic "set if this doesn't exist" method. But given that we already have concurrency issues around these Metadata classes, maybe we don't worry about this and do the "if exists" and "set" operations in two operations (we have to do this anyway for RemoteStore)

Noting that S3 recently added support for [conditional writes](https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/) (and other object stores already have this) so conceivable that s3fs/fsspec could add support for atomic writes in the near future. cc @martindurant 
TomAugspurger: > Noting that S3 recently added support for [conditional writes](https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/) (and other object stores already have this) so conceivable that s3fs/fsspec could add support for atomic writes in the near future

Opened https://github.com/fsspec/filesystem_spec/issues/1693 for that. We can pick that API up if / when it's available, but live with the potential for concurrent writers to mess each other up for now.
d-v-b: ### Should this be in `_save_metadata` or in `_create_(array / group)`
what's the reasoning for creating the intermediate groups as an optional extra behavior in `_save_metadata` versus putting it slightly higher in the stack (e.g., in the array / group creation routines)? The structure of e.g. `_create_v3` is basically "prepare metadata document, save it"; to me it seems like a natural change to expand this functionality to "prepare metadata document, save that one and all the other ones we might need". As I would naively expect the `_save_metadata` method to be scoped to only save a single metadata document, I would expect a set of documents to be saved higher in the stack, e.g. in array / group creation functions.

### Nitpicking the name `setdefault`; maybe we should give this functionality to `Store.set`?
I had to read up on the `setdefault` dict method because I had never used it before. Personally I don't find the name of that method very clear -- generally if something is called `setX(value)` I will expect it to have semantics roughly equivalent to "set the X to value", but "setdefault" doesn't "set the default to value". Maybe I'm just weird here, and for dicts a slightly unclear name is probably acceptable damage, but I don't think this is the best name for a `Store` method. I do think the naming challenge is real here, because we are trying to describe what this method _doesn't_ do, but maybe a name like `set_no_replace` would be better? But this is just personal preference in the end.  

One solution to the naming issue is to simply not introduce a new method at all -- what if we made the `setdefault` functionality part of `Store.set`, e.g. `Store.set(key, value, *, supplant / clobber / replace: bool)`? This way anything that wraps `Store.set`, e.g. `_set_many`, can support the "create if not exists" functionality.
TomAugspurger: > Should this be in _save_metadata or in _create_(array / group)

Mmm that does sound a bit cleaner. The reason for putting it in `_save_metadata` was to do all of the I/O operations concurrently. If we put it in the `_create_*` method, I think we'd have to wait for each parent creation to finish before we could write the child (recursively back to the root).

I could imagine a refactor that builds up a list of operations and hands that off to a writer. We do something like of like that in `_save_metadata` today (between `to_buffer_dict` and the `set_or_delete` stuff, which makes coroutines).

> Nitpicking the name setdefault; maybe we should give this functionality to Store.set?

Since this has the essentially the same behavior of setdefault (aside from the return value), that did feel natural to me. As a bonus, you now know about setdefault :) Some arguments either way:

1. Putting this behind a keyword in `Store.set` is totally possible. I guess there's some slight confusion between the interaction of `byte_range` and whatever new keyword we use
2. If we go with `setdefault` (or some other name) as a new method, we could in theory provide a default implementation on `Store` that works, but isn't safe for concurrent writers.

I don't have a strong preference.
TomAugspurger: Some alternative names to `setdefault`:

- `create` (but I worry that `create` should error if the thing already exists, and I'm leery of putting specific exception types into interfaces, since that's easy for implementations to mess up)
- `create_if_not_exists`
- `create_or_ignore`
jhamman: RE name, how about `set_if_not_exists`? I'm also okay with `setdefault` but I agree another name could add some clarity. 

One question about the ABC is whether we want to put a default implementation in? Something like:

```
async def set_if_not_exists(self, key, value) -> None:
    if not (await self.contains(key)):
        await self.set(key, value)
```

This is obviously not atomic but is as good as we can do for some stores.
TomAugspurger: `set_if_not_exists` works for me.

A default implementation, even if it isn't atomic, is probably OK. This isn't going to be our only source of concurrency issues between Nodes & Stores. I'll leave a comment that implementations with that ability might want to override this.
TomAugspurger: This should be good to go, maybe aside from @d-v-b's point about `_save_metadata` maybe not being the right spot for this.

I do agree that it's a bit strange to shove this in there, but IMO the benefit of creating all the parents concurrently makes it worth it. In the future I think we could refactor this a bit to have a kind of bulk `save_objects` method, but we'll still need all the create routines to build up the list of their parents and add them to the `create_if_not_exists` list, and that'll have to happen in either the `_create` or `_save_metadata` methods.

Patch:
diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py
index c4d0f9df..bd6befce 100644
--- a/src/zarr/abc/store.py
+++ b/src/zarr/abc/store.py
@@ -172,6 +172,22 @@ class Store(ABC):
         """
         ...
 
+    async def set_if_not_exists(self, key: str, value: Buffer) -> None:
+        """
+        Store a key to ``value`` if the key is not already present.
+
+        Parameters
+        -----------
+        key : str
+        value : Buffer
+        """
+        # Note for implementers: the default implementation provided here
+        # is not safe for concurrent writers. There's a race condition between
+        # the `exists` check and the `set` where another writer could set some
+        # value at `key` or delete `key`.
+        if not await self.exists(key):
+            await self.set(key, value)
+
     async def _set_many(self, values: Iterable[tuple[str, Buffer]]) -> None:
         """
         Insert multiple (key, value) pairs into storage.
@@ -297,6 +313,8 @@ class ByteSetter(Protocol):
 
     async def delete(self) -> None: ...
 
+    async def set_if_not_exists(self, default: Buffer) -> None: ...
+
 
 async def set_or_delete(byte_setter: ByteSetter, value: Buffer | None) -> None:
     if value is None:
diff --git a/src/zarr/codecs/sharding.py b/src/zarr/codecs/sharding.py
index c818e3c6..2181e9eb 100644
--- a/src/zarr/codecs/sharding.py
+++ b/src/zarr/codecs/sharding.py
@@ -97,6 +97,9 @@ class _ShardingByteSetter(_ShardingByteGetter, ByteSetter):
     async def delete(self) -> None:
         del self.shard_dict[self.chunk_coords]
 
+    async def set_if_not_exists(self, default: Buffer) -> None:
+        self.shard_dict.setdefault(self.chunk_coords, default)
+
 
 class _ShardIndex(NamedTuple):
     # dtype uint64, shape (chunks_per_shard_0, chunks_per_shard_1, ..., 2)
diff --git a/src/zarr/core/array.py b/src/zarr/core/array.py
index 25af0550..e1de15c7 100644
--- a/src/zarr/core/array.py
+++ b/src/zarr/core/array.py
@@ -13,7 +13,12 @@ from zarr.abc.store import set_or_delete
 from zarr.codecs import BytesCodec
 from zarr.codecs._v2 import V2Compressor, V2Filters
 from zarr.core.attributes import Attributes
-from zarr.core.buffer import BufferPrototype, NDArrayLike, NDBuffer, default_buffer_prototype
+from zarr.core.buffer import (
+    BufferPrototype,
+    NDArrayLike,
+    NDBuffer,
+    default_buffer_prototype,
+)
 from zarr.core.chunk_grids import RegularChunkGrid, _guess_chunks
 from zarr.core.chunk_key_encodings import (
     ChunkKeyEncoding,
@@ -71,6 +76,7 @@ if TYPE_CHECKING:
     from collections.abc import Iterable, Iterator, Sequence
 
     from zarr.abc.codec import Codec, CodecPipeline
+    from zarr.core.group import AsyncGroup
     from zarr.core.metadata.common import ArrayMetadata
 
 # Array and AsyncArray are defined in the base ``zarr`` namespace
@@ -337,7 +343,7 @@ class AsyncArray:
         )
 
         array = cls(metadata=metadata, store_path=store_path)
-        await array._save_metadata(metadata)
+        await array._save_metadata(metadata, ensure_parents=True)
         return array
 
     @classmethod
@@ -376,7 +382,7 @@ class AsyncArray:
             attributes=attributes,
         )
         array = cls(metadata=metadata, store_path=store_path)
-        await array._save_metadata(metadata)
+        await array._save_metadata(metadata, ensure_parents=True)
         return array
 
     @classmethod
@@ -621,9 +627,24 @@ class AsyncArray:
         )
         return await self._get_selection(indexer, prototype=prototype)
 
-    async def _save_metadata(self, metadata: ArrayMetadata) -> None:
+    async def _save_metadata(self, metadata: ArrayMetadata, ensure_parents: bool = False) -> None:
         to_save = metadata.to_buffer_dict(default_buffer_prototype())
         awaitables = [set_or_delete(self.store_path / key, value) for key, value in to_save.items()]
+
+        if ensure_parents:
+            # To enable zarr.create(store, path="a/b/c"), we need to create all the intermediate groups.
+            parents = _build_parents(self)
+
+            for parent in parents:
+                awaitables.extend(
+                    [
+                        (parent.store_path / key).set_if_not_exists(value)
+                        for key, value in parent.metadata.to_buffer_dict(
+                            default_buffer_prototype()
+                        ).items()
+                    ]
+                )
+
         await gather(*awaitables)
 
     async def _set_selection(
@@ -2354,3 +2375,21 @@ def chunks_initialized(array: Array | AsyncArray) -> tuple[str, ...]:
             out.append(chunk_key)
 
     return tuple(out)
+
+
+def _build_parents(node: AsyncArray | AsyncGroup) -> list[AsyncGroup]:
+    from zarr.core.group import AsyncGroup, GroupMetadata
+
+    required_parts = node.store_path.path.split("/")[:-1]
+    parents = []
+
+    for i, part in enumerate(required_parts):
+        path = "/".join(required_parts[:i] + [part])
+        parents.append(
+            AsyncGroup(
+                metadata=GroupMetadata(zarr_format=node.metadata.zarr_format),
+                store_path=StorePath(store=node.store_path.store, path=path),
+            )
+        )
+
+    return parents
diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py
index d7ad960b..79d03d3f 100644
--- a/src/zarr/core/group.py
+++ b/src/zarr/core/group.py
@@ -13,7 +13,7 @@ from typing_extensions import deprecated
 import zarr.api.asynchronous as async_api
 from zarr.abc.metadata import Metadata
 from zarr.abc.store import Store, set_or_delete
-from zarr.core.array import Array, AsyncArray
+from zarr.core.array import Array, AsyncArray, _build_parents
 from zarr.core.attributes import Attributes
 from zarr.core.buffer import default_buffer_prototype
 from zarr.core.common import (
@@ -144,7 +144,7 @@ class AsyncGroup:
             metadata=GroupMetadata(attributes=attributes, zarr_format=zarr_format),
             store_path=store_path,
         )
-        await group._save_metadata()
+        await group._save_metadata(ensure_parents=True)
         return group
 
     @classmethod
@@ -279,9 +279,22 @@ class AsyncGroup:
         else:
             raise ValueError(f"unexpected zarr_format: {self.metadata.zarr_format}")
 
-    async def _save_metadata(self) -> None:
+    async def _save_metadata(self, ensure_parents: bool = False) -> None:
         to_save = self.metadata.to_buffer_dict(default_buffer_prototype())
         awaitables = [set_or_delete(self.store_path / key, value) for key, value in to_save.items()]
+
+        if ensure_parents:
+            parents = _build_parents(self)
+            for parent in parents:
+                awaitables.extend(
+                    [
+                        (parent.store_path / key).set_if_not_exists(value)
+                        for key, value in parent.metadata.to_buffer_dict(
+                            default_buffer_prototype()
+                        ).items()
+                    ]
+                )
+
         await asyncio.gather(*awaitables)
 
     @property
diff --git a/src/zarr/store/common.py b/src/zarr/store/common.py
index c4d837e7..ea0edbe5 100644
--- a/src/zarr/store/common.py
+++ b/src/zarr/store/common.py
@@ -51,6 +51,9 @@ class StorePath:
     async def delete(self) -> None:
         await self.store.delete(self.path)
 
+    async def set_if_not_exists(self, default: Buffer) -> None:
+        await self.store.set_if_not_exists(self.path, default)
+
     async def exists(self) -> bool:
         return await self.store.exists(self.path)
 
diff --git a/src/zarr/store/local.py b/src/zarr/store/local.py
index f1bce769..23a87ea4 100644
--- a/src/zarr/store/local.py
+++ b/src/zarr/store/local.py
@@ -60,6 +60,7 @@ def _put(
     path: Path,
     value: Buffer,
     start: int | None = None,
+    exclusive: bool = False,
 ) -> int | None:
     path.parent.mkdir(parents=True, exist_ok=True)
     if start is not None:
@@ -68,7 +69,13 @@ def _put(
             f.write(value.as_numpy_array().tobytes())
         return None
     else:
-        return path.write_bytes(value.as_numpy_array().tobytes())
+        view = memoryview(value.as_numpy_array().tobytes())
+        if exclusive:
+            mode = "xb"
+        else:
+            mode = "wb"
+        with path.open(mode=mode) as f:
+            return f.write(view)
 
 
 class LocalStore(Store):
@@ -152,6 +159,15 @@ class LocalStore(Store):
         return await concurrent_map(args, to_thread, limit=None)  # TODO: fix limit
 
     async def set(self, key: str, value: Buffer) -> None:
+        return await self._set(key, value)
+
+    async def set_if_not_exists(self, key: str, value: Buffer) -> None:
+        try:
+            return await self._set(key, value, exclusive=True)
+        except FileExistsError:
+            pass
+
+    async def _set(self, key: str, value: Buffer, exclusive: bool = False) -> None:
         if not self._is_open:
             await self._open()
         self._check_writable()
@@ -159,7 +175,7 @@ class LocalStore(Store):
         if not isinstance(value, Buffer):
             raise TypeError("LocalStore.set(): `value` must a Buffer instance")
         path = self.root / key
-        await to_thread(_put, path, value)
+        await to_thread(_put, path, value, start=None, exclusive=exclusive)
 
     async def set_partial_values(
         self, key_start_values: Iterable[tuple[str, int, bytes | bytearray | memoryview]]
diff --git a/src/zarr/store/logging.py b/src/zarr/store/logging.py
index ee061417..3a4ae26c 100644
--- a/src/zarr/store/logging.py
+++ b/src/zarr/store/logging.py
@@ -8,6 +8,7 @@ from contextlib import contextmanager
 from typing import TYPE_CHECKING
 
 from zarr.abc.store import AccessMode, ByteRangeRequest, Store
+from zarr.core.buffer import Buffer
 
 if TYPE_CHECKING:
     from collections.abc import AsyncGenerator, Generator, Iterable
@@ -138,6 +139,10 @@ class LoggingStore(Store):
         with self.log():
             return await self._store.set(key=key, value=value)
 
+    async def set_if_not_exists(self, key: str, default: Buffer) -> None:
+        with self.log():
+            return await self._store.set_if_not_exists(key=key, value=default)
+
     async def delete(self, key: str) -> None:
         with self.log():
             return await self._store.delete(key=key)
diff --git a/src/zarr/store/memory.py b/src/zarr/store/memory.py
index ee4107b0..d5294c9d 100644
--- a/src/zarr/store/memory.py
+++ b/src/zarr/store/memory.py
@@ -85,9 +85,8 @@ class MemoryStore(Store):
         return key in self._store_dict
 
     async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None = None) -> None:
-        if not self._is_open:
-            await self._open()
         self._check_writable()
+        await self._ensure_open()
         assert isinstance(key, str)
         if not isinstance(value, Buffer):
             raise TypeError(f"Expected Buffer. Got {type(value)}.")
@@ -99,6 +98,11 @@ class MemoryStore(Store):
         else:
             self._store_dict[key] = value
 
+    async def set_if_not_exists(self, key: str, default: Buffer) -> None:
+        self._check_writable()
+        await self._ensure_open()
+        self._store_dict.setdefault(key, default)
+
     async def delete(self, key: str) -> None:
         self._check_writable()
         try:
diff --git a/src/zarr/store/remote.py b/src/zarr/store/remote.py
index 284cd8d7..6cc631d3 100644
--- a/src/zarr/store/remote.py
+++ b/src/zarr/store/remote.py
@@ -5,6 +5,7 @@ from typing import TYPE_CHECKING, Any
 import fsspec
 
 from zarr.abc.store import ByteRangeRequest, Store
+from zarr.core.buffer import Buffer
 from zarr.store.common import _dereference_path
 
 if TYPE_CHECKING:
diff --git a/src/zarr/store/zip.py b/src/zarr/store/zip.py
index f9c45870..82ce7d02 100644
--- a/src/zarr/store/zip.py
+++ b/src/zarr/store/zip.py
@@ -188,6 +188,13 @@ class ZipStore(Store):
     async def set_partial_values(self, key_start_values: Iterable[tuple[str, int, bytes]]) -> None:
         raise NotImplementedError
 
+    async def set_if_not_exists(self, key: str, default: Buffer) -> None:
+        self._check_writable()
+        with self._lock:
+            members = self._zf.namelist()
+            if key not in members:
+                self._set(key, default)
+
     async def delete(self, key: str) -> None:
         raise NotImplementedError
 


Test patch:
diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 5c750073..ed49936d 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -273,3 +273,19 @@ class StoreTests(Generic[S, B]):
 
         keys_observed = await _collect_aiterator(store.list_dir(root + "/"))
         assert sorted(keys_expected) == sorted(keys_observed)
+
+    async def test_set_if_not_exists(self, store: S) -> None:
+        key = "k"
+        data_buf = self.buffer_cls.from_bytes(b"0000")
+        self.set(store, key, data_buf)
+
+        new = self.buffer_cls.from_bytes(b"1111")
+        await store.set_if_not_exists("k", new)  # no error
+
+        result = await store.get(key, default_buffer_prototype())
+        assert result == data_buf
+
+        await store.set_if_not_exists("k2", new)  # no error
+
+        result = await store.get("k2", default_buffer_prototype())
+        assert result == new
diff --git a/tests/v3/test_array.py b/tests/v3/test_array.py
index 6224bc39..5de4a4d1 100644
--- a/tests/v3/test_array.py
+++ b/tests/v3/test_array.py
@@ -5,11 +5,13 @@ from typing import Literal
 import numpy as np
 import pytest
 
+import zarr.api.asynchronous
 from zarr import Array, AsyncArray, Group
 from zarr.codecs.bytes import BytesCodec
 from zarr.core.array import chunks_initialized
 from zarr.core.buffer.cpu import NDBuffer
 from zarr.core.common import JSON, ZarrFormat
+from zarr.core.group import AsyncGroup
 from zarr.core.indexing import ceildiv
 from zarr.core.sync import sync
 from zarr.errors import ContainsArrayError, ContainsGroupError
@@ -66,6 +68,47 @@ def test_array_creation_existing_node(
             )
 
 
+@pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"])
+@pytest.mark.parametrize("zarr_format", [2, 3])
+async def test_create_creates_parents(
+    store: LocalStore | MemoryStore, zarr_format: ZarrFormat
+) -> None:
+    # prepare a root node, with some data set
+    await zarr.api.asynchronous.open_group(
+        store=store, path="a", zarr_format=zarr_format, attributes={"key": "value"}
+    )
+
+    # create a child node with a couple intermediates
+    await zarr.api.asynchronous.create(
+        shape=(2, 2), store=store, path="a/b/c/d", zarr_format=zarr_format
+    )
+    parts = ["a", "a/b", "a/b/c"]
+
+    if zarr_format == 2:
+        files = [".zattrs", ".zgroup"]
+    else:
+        files = ["zarr.json"]
+
+    expected = [f"{part}/{file}" for file in files for part in parts]
+
+    if zarr_format == 2:
+        expected.append("a/b/c/d/.zarray")
+        expected.append("a/b/c/d/.zattrs")
+    else:
+        expected.append("a/b/c/d/zarr.json")
+
+    expected = sorted(expected)
+
+    result = sorted([x async for x in store.list_prefix("")])
+
+    assert result == expected
+
+    paths = ["a", "a/b", "a/b/c"]
+    for path in paths:
+        g = await zarr.api.asynchronous.open_group(store=store, path=path)
+        assert isinstance(g, AsyncGroup)
+
+
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"])
 @pytest.mark.parametrize("zarr_format", [2, 3])
 def test_array_name_properties_no_group(
diff --git a/tests/v3/test_group.py b/tests/v3/test_group.py
index 8c6464d3..cb1681da 100644
--- a/tests/v3/test_group.py
+++ b/tests/v3/test_group.py
@@ -7,6 +7,7 @@ import numpy as np
 import pytest
 
 import zarr
+import zarr.api.asynchronous
 from zarr import Array, AsyncArray, AsyncGroup, Group
 from zarr.abc.store import Store
 from zarr.core.buffer import default_buffer_prototype
@@ -56,6 +57,46 @@ def test_group_init(store: Store, zarr_format: ZarrFormat) -> None:
     assert group._async_group == agroup
 
 
+async def test_create_creates_parents(store: Store, zarr_format: ZarrFormat) -> None:
+    # prepare a root node, with some data set
+    await zarr.api.asynchronous.open_group(
+        store=store, path="a", zarr_format=zarr_format, attributes={"key": "value"}
+    )
+    # create a child node with a couple intermediates
+    await zarr.api.asynchronous.open_group(store=store, path="a/b/c/d", zarr_format=zarr_format)
+    parts = ["a", "a/b", "a/b/c"]
+
+    if zarr_format == 2:
+        files = [".zattrs", ".zgroup"]
+    else:
+        files = ["zarr.json"]
+
+    expected = [f"{part}/{file}" for file in files for part in parts]
+
+    if zarr_format == 2:
+        expected.append("a/b/c/d/.zgroup")
+        expected.append("a/b/c/d/.zattrs")
+    else:
+        expected.append("a/b/c/d/zarr.json")
+
+    expected = sorted(expected)
+
+    result = sorted([x async for x in store.list_prefix("")])
+
+    assert result == expected
+
+    paths = ["a", "a/b", "a/b/c"]
+    for path in paths:
+        g = await zarr.api.asynchronous.open_group(store=store, path=path)
+        assert isinstance(g, AsyncGroup)
+
+        if path == "a":
+            # ensure we didn't overwrite the root attributes
+            assert g.attrs == {"key": "value"}
+        else:
+            assert g.attrs == {}
+
+
 def test_group_name_properties(store: Store, zarr_format: ZarrFormat) -> None:
     """
     Test basic properties of groups
diff --git a/tests/v3/test_store/test_stateful_store.py b/tests/v3/test_store/test_stateful_store.py
index 1a43b55f..efa1953a 100644
--- a/tests/v3/test_store/test_stateful_store.py
+++ b/tests/v3/test_store/test_stateful_store.py
@@ -239,7 +239,7 @@ class ZarrStoreStateMachine(RuleBasedStateMachine):
 
 
 def test_zarr_hierarchy(sync_store: Store) -> None:
-    def mk_test_instance_sync():
+    def mk_test_instance_sync() -> None:
         return ZarrStoreStateMachine(sync_store)
 
     if isinstance(sync_store, ZipStore):