From 2ea442c45c6327b24202dca67ca595b589df5a77 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 10:22:00 -0500
Subject: [PATCH 01/44] Run Store tests on logging

---
 tests/test_store/test_logging.py | 34 +++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index b32a214db..755be7b1f 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -5,13 +5,45 @@
 import pytest
 
 import zarr
-from zarr.core.buffer import default_buffer_prototype
+from zarr.core.buffer import Buffer, cpu, default_buffer_prototype
 from zarr.storage import LoggingStore
+from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
     from zarr.abc.store import Store
 
 
+class TestLoggingStore(StoreTests[LoggingStore, cpu.Buffer]):
+    store_cls = LoggingStore
+    buffer_cls = cpu.Buffer
+
+    async def get(self, store: LoggingStore, key: str) -> Buffer:
+        return self.buffer_cls.from_bytes((store._store.root / key).read_bytes())
+
+    async def set(self, store: LoggingStore, key: str, value: Buffer) -> None:
+        parent = (store._store.root / key).parent
+        if not parent.exists():
+            parent.mkdir(parents=True)
+        (store._store.root / key).write_bytes(value.to_bytes())
+
+    @pytest.fixture
+    def store_kwargs(self, local_store) -> dict[str, str]:
+        return {"store": local_store, "log_level": "DEBUG"}
+
+    @pytest.fixture
+    def store(self, store_kwargs: str | dict[str, Buffer] | None) -> LoggingStore:
+        return self.store_cls(**store_kwargs)
+
+    def test_store_supports_writes(self, store: LoggingStore) -> None:
+        assert store.supports_writes
+
+    def test_store_supports_partial_writes(self, store: LoggingStore) -> None:
+        assert store.supports_partial_writes
+
+    def test_store_supports_listing(self, store: LoggingStore) -> None:
+        assert store.supports_listing
+
+
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"])
 async def test_logging_store(store: Store, caplog) -> None:
     wrapped = LoggingStore(store=store, log_level="DEBUG")

From 7f7657565f6d7a0b7a6d5be9b8dbe1819e0667f2 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 10:22:15 -0500
Subject: [PATCH 02/44] Run store tests on wrapper

---
 tests/test_store/test_wrapper.py | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 489bcd5a7..da341296a 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -6,12 +6,44 @@
 
 from zarr.core.buffer.cpu import Buffer, buffer_prototype
 from zarr.storage import WrapperStore
+from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
     from zarr.abc.store import Store
     from zarr.core.buffer.core import BufferPrototype
 
 
+class TestWrapperStore(StoreTests[WrapperStore, Buffer]):
+    store_cls = WrapperStore
+    buffer_cls = Buffer
+
+    async def get(self, store: WrapperStore, key: str) -> Buffer:
+        return self.buffer_cls.from_bytes((store._store.root / key).read_bytes())
+
+    async def set(self, store: WrapperStore, key: str, value: Buffer) -> None:
+        parent = (store._store.root / key).parent
+        if not parent.exists():
+            parent.mkdir(parents=True)
+        (store._store.root / key).write_bytes(value.to_bytes())
+
+    @pytest.fixture
+    def store_kwargs(self, local_store) -> dict[str, str]:
+        return {"store": local_store}
+
+    @pytest.fixture
+    def store(self, store_kwargs: str | dict[str, Buffer] | None) -> WrapperStore:
+        return self.store_cls(**store_kwargs)
+
+    def test_store_supports_writes(self, store: WrapperStore) -> None:
+        assert store.supports_writes
+
+    def test_store_supports_partial_writes(self, store: WrapperStore) -> None:
+        assert store.supports_partial_writes
+
+    def test_store_supports_listing(self, store: WrapperStore) -> None:
+        assert store.supports_listing
+
+
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)
 async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None:
     # define a class that prints when it sets

From 98b739227c0048a3060e7da1d8720575cad7375f Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 14:09:13 -0500
Subject: [PATCH 03/44] Add read only open tests to WrapperStore

---
 src/zarr/storage/_wrapper.py     |  8 +++++++
 tests/test_store/test_wrapper.py | 38 +++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
index 255e96543..43b10f15e 100644
--- a/src/zarr/storage/_wrapper.py
+++ b/src/zarr/storage/_wrapper.py
@@ -56,6 +56,14 @@ async def _ensure_open(self) -> None:
     async def is_empty(self, prefix: str) -> bool:
         return await self._store.is_empty(prefix)
 
+    @property
+    def _is_open(self) -> bool:
+        return self._store._is_open
+
+    @_is_open.setter
+    def _is_open(self, value: bool) -> None:
+        self._store._is_open = value
+
     async def clear(self) -> None:
         return await self._store.clear()
 
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index da341296a..86d5df24b 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -5,10 +5,12 @@
 import pytest
 
 from zarr.core.buffer.cpu import Buffer, buffer_prototype
-from zarr.storage import WrapperStore
+from zarr.storage import LocalStore, WrapperStore
 from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
+    from typing import Any
+
     from zarr.abc.store import Store
     from zarr.core.buffer.core import BufferPrototype
 
@@ -43,6 +45,40 @@ def test_store_supports_partial_writes(self, store: WrapperStore) -> None:
     def test_store_supports_listing(self, store: WrapperStore) -> None:
         assert store.supports_listing
 
+    @pytest.mark.parametrize("read_only", [True, False])
+    async def test_store_open_read_only(
+        self, store_kwargs: dict[str, Any], read_only: bool, tmpdir
+    ) -> None:
+        store_kwargs = {
+            **store_kwargs,
+            "read_only": read_only,
+            "root": str(tmpdir),
+            "store_cls": LocalStore,
+        }
+        store_kwargs.pop("store")
+        store = await self.store_cls.open(**store_kwargs)
+        assert store._is_open
+        assert store.read_only == read_only
+
+    async def test_read_only_store_raises(self, store_kwargs: dict[str, Any], tmpdir) -> None:
+        store_kwargs = {
+            **store_kwargs,
+            "read_only": True,
+            "root": str(tmpdir),
+            "store_cls": LocalStore,
+        }
+        store_kwargs.pop("store")
+        store = await self.store_cls.open(**store_kwargs)
+        assert store.read_only
+
+        # set
+        with pytest.raises(ValueError):
+            await store.set("foo", self.buffer_cls.from_bytes(b"bar"))
+
+        # delete
+        with pytest.raises(ValueError):
+            await store.delete("foo")
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)
 async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None:

From 18be47facf3d31fd4c9f6c09d1052e4b46df5788 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 14:09:33 -0500
Subject: [PATCH 04/44] Ignore new coverage files

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 5663f62d0..1b2b63e65 100644
--- a/.gitignore
+++ b/.gitignore
@@ -83,6 +83,7 @@ src/zarr/_version.py
 data/*
 src/fixture/
 fixture/
+junit.xml
 
 .DS_Store
 tests/.hypothesis

From 69ce1d7e9943feb481e9733b9ceedef0ef26f8ca Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 14:24:44 -0500
Subject: [PATCH 05/44] Simplify wrapper tests

---
 tests/test_store/test_wrapper.py | 46 +++++---------------------------
 1 file changed, 7 insertions(+), 39 deletions(-)

diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 86d5df24b..08856da54 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -9,8 +9,6 @@
 from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
-    from typing import Any
-
     from zarr.abc.store import Store
     from zarr.core.buffer.core import BufferPrototype
 
@@ -29,12 +27,16 @@ async def set(self, store: WrapperStore, key: str, value: Buffer) -> None:
         (store._store.root / key).write_bytes(value.to_bytes())
 
     @pytest.fixture
-    def store_kwargs(self, local_store) -> dict[str, str]:
+    def init_kwargs(self, local_store) -> dict[str, str]:
         return {"store": local_store}
 
     @pytest.fixture
-    def store(self, store_kwargs: str | dict[str, Buffer] | None) -> WrapperStore:
-        return self.store_cls(**store_kwargs)
+    def store_kwargs(self, tmpdir) -> dict[str, str]:
+        return {"store_cls": LocalStore, "root": str(tmpdir)}
+
+    @pytest.fixture
+    def store(self, init_kwargs: str | dict[str, str] | None) -> WrapperStore:
+        return self.store_cls(**init_kwargs)
 
     def test_store_supports_writes(self, store: WrapperStore) -> None:
         assert store.supports_writes
@@ -45,40 +47,6 @@ def test_store_supports_partial_writes(self, store: WrapperStore) -> None:
     def test_store_supports_listing(self, store: WrapperStore) -> None:
         assert store.supports_listing
 
-    @pytest.mark.parametrize("read_only", [True, False])
-    async def test_store_open_read_only(
-        self, store_kwargs: dict[str, Any], read_only: bool, tmpdir
-    ) -> None:
-        store_kwargs = {
-            **store_kwargs,
-            "read_only": read_only,
-            "root": str(tmpdir),
-            "store_cls": LocalStore,
-        }
-        store_kwargs.pop("store")
-        store = await self.store_cls.open(**store_kwargs)
-        assert store._is_open
-        assert store.read_only == read_only
-
-    async def test_read_only_store_raises(self, store_kwargs: dict[str, Any], tmpdir) -> None:
-        store_kwargs = {
-            **store_kwargs,
-            "read_only": True,
-            "root": str(tmpdir),
-            "store_cls": LocalStore,
-        }
-        store_kwargs.pop("store")
-        store = await self.store_cls.open(**store_kwargs)
-        assert store.read_only
-
-        # set
-        with pytest.raises(ValueError):
-            await store.set("foo", self.buffer_cls.from_bytes(b"bar"))
-
-        # delete
-        with pytest.raises(ValueError):
-            await store.delete("foo")
-
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)
 async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None:

From 5877355f2ec65ce04113eac739e93212095988a8 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 14:40:12 -0500
Subject: [PATCH 06/44] Fix __eq__ method in WrapperStore

---
 src/zarr/storage/_wrapper.py     |  2 +-
 tests/test_store/test_wrapper.py | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
index 43b10f15e..6a6341cdb 100644
--- a/src/zarr/storage/_wrapper.py
+++ b/src/zarr/storage/_wrapper.py
@@ -75,7 +75,7 @@ def _check_writable(self) -> None:
         return self._store._check_writable()
 
     def __eq__(self, value: object) -> bool:
-        return type(self) is type(value) and self._store.__eq__(value)
+        return type(self) is type(value) and self._store.__eq__(value._store)  # type: ignore[attr-defined]
 
     async def get(
         self, key: str, prototype: BufferPrototype, byte_range: ByteRequest | None = None
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 08856da54..ce2406f48 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -35,7 +35,7 @@ def store_kwargs(self, tmpdir) -> dict[str, str]:
         return {"store_cls": LocalStore, "root": str(tmpdir)}
 
     @pytest.fixture
-    def store(self, init_kwargs: str | dict[str, str] | None) -> WrapperStore:
+    def store(self, init_kwargs: dict[str, str]) -> WrapperStore:
         return self.store_cls(**init_kwargs)
 
     def test_store_supports_writes(self, store: WrapperStore) -> None:
@@ -47,6 +47,16 @@ def test_store_supports_partial_writes(self, store: WrapperStore) -> None:
     def test_store_supports_listing(self, store: WrapperStore) -> None:
         assert store.supports_listing
 
+    def test_store_eq(self, store: WrapperStore, init_kwargs: dict[str, str]) -> None:
+        # Overload store.testing version due to different store.open and store.__init__ kwargs
+        # check self equality
+        assert store == store
+
+        # check store equality with same inputs
+        # asserting this is important for being able to compare (de)serialized stores
+        store2 = self.store_cls(**init_kwargs)
+        assert store == store2
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)
 async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None:

From b4310fdaad7ca058d4ac9cba70cc3b61fd5f2df4 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 14:46:29 -0500
Subject: [PATCH 07/44] Implement __repr__ for WrapperStore

---
 src/zarr/storage/_wrapper.py     | 3 +++
 tests/test_store/test_wrapper.py | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
index 6a6341cdb..14af5c1c4 100644
--- a/src/zarr/storage/_wrapper.py
+++ b/src/zarr/storage/_wrapper.py
@@ -77,6 +77,9 @@ def _check_writable(self) -> None:
     def __eq__(self, value: object) -> bool:
         return type(self) is type(value) and self._store.__eq__(value._store)  # type: ignore[attr-defined]
 
+    def __repr__(self) -> str:
+        return f"WrapperStore({repr(self._store)!r})"
+
     async def get(
         self, key: str, prototype: BufferPrototype, byte_range: ByteRequest | None = None
     ) -> Buffer | None:
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index ce2406f48..52260dcab 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -57,6 +57,11 @@ def test_store_eq(self, store: WrapperStore, init_kwargs: dict[str, str]) -> Non
         store2 = self.store_cls(**init_kwargs)
         assert store == store2
 
+    def test_store_repr(self, store: WrapperStore) -> None:
+        assert (
+            str(store) == f"WrapperStore(\"LocalStore('file://{store._store.root.as_posix()}')\")"
+        )
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)
 async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None:

From d08458ed1d81c49058b9a99ae42b52bf53246791 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 15:02:04 -0500
Subject: [PATCH 08/44] Allow separate open and init kwargs

---
 src/zarr/testing/store.py        | 20 +++++++++++---------
 tests/test_store/test_wrapper.py | 18 ++++--------------
 2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 602d00169..c89faeeea 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -59,8 +59,12 @@ def store_kwargs(self) -> dict[str, Any]:
         return {"read_only": False}
 
     @pytest.fixture
-    async def store(self, store_kwargs: dict[str, Any]) -> Store:
-        return await self.store_cls.open(**store_kwargs)
+    def open_kwargs(self, store_kwargs: dict[str, Any]) -> dict[str, Any]:
+        return store_kwargs
+
+    @pytest.fixture
+    async def store(self, open_kwargs: dict[str, Any]) -> Store:
+        return await self.store_cls.open(**open_kwargs)
 
     def test_store_type(self, store: S) -> None:
         assert isinstance(store, Store)
@@ -86,16 +90,14 @@ def test_store_read_only(self, store: S) -> None:
             store.read_only = False  # type: ignore[misc]
 
     @pytest.mark.parametrize("read_only", [True, False])
-    async def test_store_open_read_only(
-        self, store_kwargs: dict[str, Any], read_only: bool
-    ) -> None:
-        store_kwargs["read_only"] = read_only
-        store = await self.store_cls.open(**store_kwargs)
+    async def test_store_open_read_only(self, open_kwargs: dict[str, Any], read_only: bool) -> None:
+        open_kwargs["read_only"] = read_only
+        store = await self.store_cls.open(**open_kwargs)
         assert store._is_open
         assert store.read_only == read_only
 
-    async def test_read_only_store_raises(self, store_kwargs: dict[str, Any]) -> None:
-        kwargs = {**store_kwargs, "read_only": True}
+    async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None:
+        kwargs = {**open_kwargs, "read_only": True}
         store = await self.store_cls.open(**kwargs)
         assert store.read_only
 
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 52260dcab..f37c11d26 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -27,16 +27,16 @@ async def set(self, store: WrapperStore, key: str, value: Buffer) -> None:
         (store._store.root / key).write_bytes(value.to_bytes())
 
     @pytest.fixture
-    def init_kwargs(self, local_store) -> dict[str, str]:
+    def store_kwargs(self, local_store) -> dict[str, str]:
         return {"store": local_store}
 
     @pytest.fixture
-    def store_kwargs(self, tmpdir) -> dict[str, str]:
+    def open_kwargs(self, tmpdir) -> dict[str, str]:
         return {"store_cls": LocalStore, "root": str(tmpdir)}
 
     @pytest.fixture
-    def store(self, init_kwargs: dict[str, str]) -> WrapperStore:
-        return self.store_cls(**init_kwargs)
+    def store(self, store_kwargs: dict[str, str]) -> WrapperStore:
+        return self.store_cls(**store_kwargs)
 
     def test_store_supports_writes(self, store: WrapperStore) -> None:
         assert store.supports_writes
@@ -47,16 +47,6 @@ def test_store_supports_partial_writes(self, store: WrapperStore) -> None:
     def test_store_supports_listing(self, store: WrapperStore) -> None:
         assert store.supports_listing
 
-    def test_store_eq(self, store: WrapperStore, init_kwargs: dict[str, str]) -> None:
-        # Overload store.testing version due to different store.open and store.__init__ kwargs
-        # check self equality
-        assert store == store
-
-        # check store equality with same inputs
-        # asserting this is important for being able to compare (de)serialized stores
-        store2 = self.store_cls(**init_kwargs)
-        assert store == store2
-
     def test_store_repr(self, store: WrapperStore) -> None:
         assert (
             str(store) == f"WrapperStore(\"LocalStore('file://{store._store.root.as_posix()}')\")"

From f6636945457e5e2065f3ee7c1ffc09afd6422a5c Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 15:18:43 -0500
Subject: [PATCH 09/44] Add open class method to LoggingStore

---
 src/zarr/storage/_logging.py     | 12 +++++++++++-
 tests/test_store/test_logging.py |  6 +++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py
index 5ca716df2..6bef20495 100644
--- a/src/zarr/storage/_logging.py
+++ b/src/zarr/storage/_logging.py
@@ -5,7 +5,7 @@
 import time
 from collections import defaultdict
 from contextlib import contextmanager
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, Self, TypeVar
 
 from zarr.abc.store import Store
 from zarr.storage._wrapper import WrapperStore
@@ -18,6 +18,8 @@
 
     counter: defaultdict[str, int]
 
+T_Store = TypeVar("T_Store", bound=Store)
+
 
 class LoggingStore(WrapperStore[Store]):
     """
@@ -94,6 +96,14 @@ def log(self, hint: Any = "") -> Generator[None, None, None]:
             end_time = time.time()
             self.logger.info("Finished %s [%.2f s]", op, end_time - start_time)
 
+    @classmethod
+    async def open(cls: type[Self], store_cls: type[T_Store], *args: Any, **kwargs: Any) -> Self:
+        log_level = kwargs.pop("log_level", "DEBUG")
+        log_handler = kwargs.pop("log_handler", None)
+        store = store_cls(*args, **kwargs)
+        await store._open()
+        return cls(store=store, log_level=log_level, log_handler=log_handler)
+
     @property
     def supports_writes(self) -> bool:
         with self.log():
diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index 755be7b1f..9d58105ef 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -6,7 +6,7 @@
 
 import zarr
 from zarr.core.buffer import Buffer, cpu, default_buffer_prototype
-from zarr.storage import LoggingStore
+from zarr.storage import LocalStore, LoggingStore
 from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
@@ -30,6 +30,10 @@ async def set(self, store: LoggingStore, key: str, value: Buffer) -> None:
     def store_kwargs(self, local_store) -> dict[str, str]:
         return {"store": local_store, "log_level": "DEBUG"}
 
+    @pytest.fixture
+    def open_kwargs(self, tmpdir) -> dict[str, str]:
+        return {"store_cls": LocalStore, "root": str(tmpdir), "log_level": "DEBUG"}
+
     @pytest.fixture
     def store(self, store_kwargs: str | dict[str, Buffer] | None) -> LoggingStore:
         return self.store_cls(**store_kwargs)

From cf62f676f70c4afdd75af86bc244b1a9d41f7318 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 15:24:49 -0500
Subject: [PATCH 10/44] Add __str__ to WrapperStore

---
 src/zarr/storage/_wrapper.py     | 3 +++
 tests/test_store/test_wrapper.py | 4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
index 14af5c1c4..002e1e73c 100644
--- a/src/zarr/storage/_wrapper.py
+++ b/src/zarr/storage/_wrapper.py
@@ -77,6 +77,9 @@ def _check_writable(self) -> None:
     def __eq__(self, value: object) -> bool:
         return type(self) is type(value) and self._store.__eq__(value._store)  # type: ignore[attr-defined]
 
+    def __str__(self) -> str:
+        return f"wrapping-{self._store}"
+
     def __repr__(self) -> str:
         return f"WrapperStore({repr(self._store)!r})"
 
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index f37c11d26..00151b668 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -48,9 +48,7 @@ def test_store_supports_listing(self, store: WrapperStore) -> None:
         assert store.supports_listing
 
     def test_store_repr(self, store: WrapperStore) -> None:
-        assert (
-            str(store) == f"WrapperStore(\"LocalStore('file://{store._store.root.as_posix()}')\")"
-        )
+        assert str(store) == f"wrapping-file://{store._store.root.as_posix()}"
 
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)

From 31f99313a544a8f137b90e54e39cb3f460696f22 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 15:26:06 -0500
Subject: [PATCH 11/44] Add repr test for LoggingStore

---
 tests/test_store/test_logging.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index 9d58105ef..0a0a87fe5 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -47,6 +47,9 @@ def test_store_supports_partial_writes(self, store: LoggingStore) -> None:
     def test_store_supports_listing(self, store: LoggingStore) -> None:
         assert store.supports_listing
 
+    def test_store_repr(self, store: LoggingStore) -> None:
+        assert str(store) == f"logging-file://{store._store.root.as_posix()}"
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"])
 async def test_logging_store(store: Store, caplog) -> None:

From 964aeaaf75bfb1e3d2faf8ee49ad1b7142885e7c Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 15:34:44 -0500
Subject: [PATCH 12/44] Fix __eq__ in LoggingStore

---
 src/zarr/storage/_logging.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py
index 6bef20495..8ea4deca5 100644
--- a/src/zarr/storage/_logging.py
+++ b/src/zarr/storage/_logging.py
@@ -165,7 +165,7 @@ def __repr__(self) -> str:
 
     def __eq__(self, other: object) -> bool:
         with self.log(other):
-            return self._store == other
+            return type(self) is type(other) and self._store.__eq__(other._store)  # type: ignore[attr-defined]
 
     async def get(
         self,

From 332f564da5f016076afbfb7421e558f7b87e40ca Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 15:53:29 -0500
Subject: [PATCH 13/44] Test getsize for stores

---
 src/zarr/testing/store.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index c89faeeea..88ddbb538 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -159,6 +159,25 @@ async def test_get_many(self, store: S) -> None:
         expected_kvs = sorted(((k, b) for k, b in zip(keys, values, strict=False)))
         assert observed_kvs == expected_kvs
 
+    @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"])
+    @pytest.mark.parametrize("data", [b"\x01\x02\x03\x04", b""])
+    async def test_getsize(self, store: S, key: str, data: bytes) -> None:
+        """
+        Test the result of store.getsize().
+        """
+        data_buf = self.buffer_cls.from_bytes(data)
+        expected = len(data_buf)
+        await self.set(store, key, data_buf)
+        observed = await store.getsize(key)
+        assert observed == expected
+
+    async def test_getsize_raises(self, store: S) -> None:
+        """
+        Test the result of store.getsize().
+        """
+        with pytest.raises(FileNotFoundError):
+            await store.getsize("c/1000")
+
     @pytest.mark.parametrize("key", ["zarr.json", "c/0", "foo/c/0.0", "foo/0/0"])
     @pytest.mark.parametrize("data", [b"\x01\x02\x03\x04", b""])
     async def test_set(self, store: S, key: str, data: bytes) -> None:

From 4d4d728df178fd2a12dc1783e65841a64c561942 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sat, 11 Jan 2025 16:13:17 -0500
Subject: [PATCH 14/44] Test for invalid ByteRequest

---
 src/zarr/testing/store.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 88ddbb538..e7b27ed70 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -137,6 +137,15 @@ async def test_get(self, store: S, key: str, data: bytes, byte_range: ByteReques
         expected = data_buf[start:stop]
         assert_bytes_equal(observed, expected)
 
+    async def test_get_raises(self, store: S) -> None:
+        """
+        Ensure that a ValueError is raise for invalid byte range syntax
+        """
+        data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
+        await self.set(store, "c/0", data_buf)
+        with pytest.raises((ValueError, TypeError)):
+            await store.get("c/0", prototype=default_buffer_prototype(), byte_range=(0, 2))  # type: ignore[arg-type]
+
     async def test_get_many(self, store: S) -> None:
         """
         Ensure that multiple keys can be retrieved at once with the _get_many method.

From 30d1323c531b4e6e1e08b9f771f9a063bce0f39c Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 16:29:10 -0500
Subject: [PATCH 15/44] Use stdout rather than stderr as the default logging
 stream

---
 src/zarr/storage/_logging.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py
index 8ea4deca5..d5b3b9327 100644
--- a/src/zarr/storage/_logging.py
+++ b/src/zarr/storage/_logging.py
@@ -2,6 +2,7 @@
 
 import inspect
 import logging
+import sys
 import time
 from collections import defaultdict
 from contextlib import contextmanager
@@ -69,7 +70,7 @@ def _configure_logger(
 
     def _default_handler(self) -> logging.Handler:
         """Define a default log handler"""
-        handler = logging.StreamHandler()
+        handler = logging.StreamHandler(stream=sys.stdout)
         handler.setLevel(self.log_level)
         handler.setFormatter(
             logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")

From 976420402844651a6c1754f8b591db1df51f7aa2 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 16:31:29 -0500
Subject: [PATCH 16/44] Test default logging stream

---
 tests/test_store/test_logging.py | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index 0a0a87fe5..9bf0846e3 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import logging
 from typing import TYPE_CHECKING
 
 import pytest
@@ -50,6 +51,24 @@ def test_store_supports_listing(self, store: LoggingStore) -> None:
     def test_store_repr(self, store: LoggingStore) -> None:
         assert str(store) == f"logging-file://{store._store.root.as_posix()}"
 
+    async def test_default_handler(self, local_store, capsys) -> None:
+        # Store and then remove existing handlers to enter default handler code path
+        handlers = logging.getLogger().handlers[:]
+        for h in handlers:
+            logging.getLogger().removeHandler(h)
+        # Test logs are sent to stdout
+        wrapped = LoggingStore(store=local_store)
+        buffer = default_buffer_prototype().buffer
+        res = await wrapped.set("foo/bar/c/0", buffer.from_bytes(b"\x01\x02\x03\x04"))
+        assert res is None
+        captured = capsys.readouterr()
+        assert len(captured) == 2
+        assert "Calling LocalStore.set" in captured.out
+        assert "Finished LocalStore.set" in captured.out
+        # Restore handlers
+        for h in handlers:
+            logging.getLogger().addHandler(h)
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"])
 async def test_logging_store(store: Store, caplog) -> None:

From fefd666b945d4bcdd84b3a64b07cb732f562289e Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 16:53:53 -0500
Subject: [PATCH 17/44] Add test for getsize_prefix

---
 src/zarr/testing/store.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index e7b27ed70..775a44793 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -180,6 +180,18 @@ async def test_getsize(self, store: S, key: str, data: bytes) -> None:
         observed = await store.getsize(key)
         assert observed == expected
 
+    async def test_getsize_prefix(self, store: S) -> None:
+        """
+        Test the result of store.getsize_prefix().
+        """
+        data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
+        keys = ["c/0/0", "c/0/1", "c/1/0", "c/1/1"]
+        keys_values = [(k, data_buf) for k in keys]
+        await store._set_many(keys_values)
+        expected = len(data_buf) * len(keys)
+        observed = await store.getsize_prefix("c")
+        assert observed == expected
+
     async def test_getsize_raises(self, store: S) -> None:
         """
         Test the result of store.getsize().

From 6f240c281b32a0e50ed83bd541a5e13dee6c0dc5 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 17:06:07 -0500
Subject: [PATCH 18/44] Document buffer prototype parameter

---
 src/zarr/abc/store.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/zarr/abc/store.py b/src/zarr/abc/store.py
index e6a5518a4..96165f8ba 100644
--- a/src/zarr/abc/store.py
+++ b/src/zarr/abc/store.py
@@ -176,10 +176,10 @@ async def get(
         Parameters
         ----------
         key : str
+        prototype : BufferPrototype
+            The prototype of the output buffer. Stores may support a default buffer prototype.
         byte_range : ByteRequest, optional
-
             ByteRequest may be one of the following. If not provided, all data associated with the key is retrieved.
-
             - RangeByteRequest(int, int): Request a specific range of bytes in the form (start, end). The end is exclusive. If the given range is zero-length or starts after the end of the object, an error will be returned. Additionally, if the range ends after the end of the object, the entire remainder of the object will be returned. Otherwise, the exact requested range will be returned.
             - OffsetByteRequest(int): Request all bytes starting from a given byte offset. This is equivalent to bytes={int}- as an HTTP header.
             - SuffixByteRequest(int): Request the last int bytes. Note that here, int is the size of the request, not the byte offset. This is equivalent to bytes=-{int} as an HTTP header.
@@ -200,6 +200,8 @@ async def get_partial_values(
 
         Parameters
         ----------
+        prototype : BufferPrototype
+            The prototype of the output buffer. Stores may support a default buffer prototype.
         key_ranges : Iterable[tuple[str, tuple[int | None, int | None]]]
             Ordered set of key, range pairs, a key may occur multiple times with different ranges
 

From d2bbd9d719ae633bd5afb16baf6ee4bc3986cfbd Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 17:21:18 -0500
Subject: [PATCH 19/44] Add test for invalid modes in StorePath.open()

---
 tests/test_store/test_core.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index 7806f3ece..9b9253a6b 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -56,10 +56,19 @@ async def test_make_store_path_store_path(
     assert Path(store_path.store.root) == Path(tmpdir)
     path_normalized = normalize_path(path)
     assert store_path.path == (store_like / path_normalized).path
-
     assert store_path.read_only == ro
 
 
+async def test_store_path_invalid_mode_raises(tmpdir: LEGACY_PATH) -> None:
+    """
+    Test that ValueErrors are raise for invalid mode.
+    """
+    with pytest.raises(ValueError):
+        await StorePath.open(LocalStore(str(tmpdir), read_only=True), path=None, mode="w")
+    with pytest.raises(ValueError):
+        await StorePath.open(LocalStore(str(tmpdir), read_only=False), path=None, mode="x")
+
+
 async def test_make_store_path_invalid() -> None:
     """
     Test that invalid types raise TypeError

From 85f44dbf254a3d9a8c66c7825a10814009cd29ac Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 18:05:35 -0500
Subject: [PATCH 20/44] Add test for contains_group

---
 tests/test_store/test_core.py | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index 9b9253a6b..b7c389403 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -4,12 +4,37 @@
 import pytest
 from _pytest.compat import LEGACY_PATH
 
-from zarr.core.common import AccessModeLiteral
+from zarr import Group
+from zarr.core.common import AccessModeLiteral, ZarrFormat
 from zarr.storage import FsspecStore, LocalStore, MemoryStore, StoreLike, StorePath
-from zarr.storage._common import make_store_path
+from zarr.storage._common import contains_array, contains_group, make_store_path
 from zarr.storage._utils import normalize_path
 
 
+@pytest.mark.parametrize("write_group", [True, False])
+@pytest.mark.parametrize("zarr_format", [2, 3])
+async def test_contains_group(local_store, write_group: bool, zarr_format: ZarrFormat) -> None:
+    """
+    Test contains group method
+    """
+    root = Group.from_store(store=local_store, zarr_format=zarr_format)
+    if write_group:
+        root.create_group("foo")
+    store_path = StorePath(local_store, path="foo")
+    assert await contains_group(store_path, zarr_format=zarr_format) == write_group
+
+
+async def test_contains_invalid_format_raises(local_store) -> None:
+    """
+    Test contains_group and contains_array raise errors for invalid zarr_formats
+    """
+    store_path = StorePath(local_store)
+    with pytest.raises(ValueError):
+        assert await contains_group(store_path, zarr_format="3.0")
+    with pytest.raises(ValueError):
+        assert await contains_array(store_path, zarr_format="3.0")
+
+
 @pytest.mark.parametrize("path", [None, "", "bar"])
 async def test_make_store_path_none(path: str) -> None:
     """

From 51c0c1579c1cf958a3fdc523365e3b493e1b20fb Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 18:15:11 -0500
Subject: [PATCH 21/44] Add tests for contains_array

---
 tests/test_store/test_core.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index b7c389403..806eaf9bb 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -24,6 +24,19 @@ async def test_contains_group(local_store, write_group: bool, zarr_format: ZarrF
     assert await contains_group(store_path, zarr_format=zarr_format) == write_group
 
 
+@pytest.mark.parametrize("write_array", [True, False])
+@pytest.mark.parametrize("zarr_format", [2, 3])
+async def test_contains_array(local_store, write_array: bool, zarr_format: ZarrFormat) -> None:
+    """
+    Test contains group method
+    """
+    root = Group.from_store(store=local_store, zarr_format=zarr_format)
+    if write_array:
+        root.create_array("foo", shape=(100,), chunks=(10,), dtype="i4")
+    store_path = StorePath(local_store, path="foo")
+    assert await contains_array(store_path, zarr_format=zarr_format) == write_array
+
+
 async def test_contains_invalid_format_raises(local_store) -> None:
     """
     Test contains_group and contains_array raise errors for invalid zarr_formats

From ddd6bc96459aaaa09e509f2f8c94ecf1601ba3d5 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 18:37:34 -0500
Subject: [PATCH 22/44] Test for invalid root type for LocalStore

---
 tests/test_store/test_local.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py
index 22597a2c3..714940718 100644
--- a/tests/test_store/test_local.py
+++ b/tests/test_store/test_local.py
@@ -53,3 +53,7 @@ def test_creates_new_directory(self, tmp_path: pathlib.Path):
 
         store = self.store_cls(root=target)
         zarr.group(store=store)
+
+    def test_invalid_root_raises(self):
+        with pytest.raises(TypeError):
+            LocalStore(root=0)

From 62a528c086a7e9f9a5bb33ef274ccebf1514759f Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 18:41:57 -0500
Subject: [PATCH 23/44] Test LocalStore.get with default prototype

---
 tests/test_store/test_local.py | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py
index 714940718..524f2b378 100644
--- a/tests/test_store/test_local.py
+++ b/tests/test_store/test_local.py
@@ -8,6 +8,7 @@
 from zarr.core.buffer import Buffer, cpu
 from zarr.storage import LocalStore
 from zarr.testing.store import StoreTests
+from zarr.testing.utils import assert_bytes_equal
 
 if TYPE_CHECKING:
     import pathlib
@@ -57,3 +58,13 @@ def test_creates_new_directory(self, tmp_path: pathlib.Path):
     def test_invalid_root_raises(self):
         with pytest.raises(TypeError):
             LocalStore(root=0)
+
+    async def test_get_with_prototype_default(self, store: LocalStore):
+        """
+        Ensure that data can be read using the default prototype method.
+        """
+        data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
+        key = "c/0"
+        await self.set(store, key, data_buf)
+        observed = await store.get(key, prototype=None)
+        assert_bytes_equal(observed, data_buf)

From 5f00efd38cbc55b851d71adbf04e2cc7058ff20f Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 19:12:18 -0500
Subject: [PATCH 24/44] Test for invalid set buffer arguments

---
 src/zarr/storage/_fsspec.py | 5 ++++-
 src/zarr/testing/store.py   | 7 +++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py
index 99c8c778e..0f6bced8f 100644
--- a/src/zarr/storage/_fsspec.py
+++ b/src/zarr/storage/_fsspec.py
@@ -10,6 +10,7 @@
     Store,
     SuffixByteRequest,
 )
+from zarr.core.buffer import Buffer
 from zarr.storage._common import _dereference_path
 
 if TYPE_CHECKING:
@@ -17,7 +18,7 @@
 
     from fsspec.asyn import AsyncFileSystem
 
-    from zarr.core.buffer import Buffer, BufferPrototype
+    from zarr.core.buffer import BufferPrototype
     from zarr.core.common import BytesLike
 
 
@@ -253,6 +254,8 @@ async def set(
         if not self._is_open:
             await self._open()
         self._check_writable()
+        if not isinstance(value, Buffer):
+            raise TypeError("FsspecStore.set(): `value` must a Buffer instance")
         path = _dereference_path(self.path, key)
         # write data
         if byte_range:
diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 775a44793..2c27d401d 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -223,6 +223,13 @@ async def test_set_many(self, store: S) -> None:
         for k, v in store_dict.items():
             assert (await self.get(store, k)).to_bytes() == v.to_bytes()
 
+    async def test_set_raises(self, store: S) -> None:
+        """
+        Ensure that set raises a Type or Value Error for invalid buffer arguments.
+        """
+        with pytest.raises((ValueError, TypeError)):
+            await store.set("c/0", 0)  # type: ignore[arg-type]
+
     @pytest.mark.parametrize(
         "key_ranges",
         [

From 69233372a7cdba3dc5c0b45d259e2726f6320019 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 19:39:21 -0500
Subject: [PATCH 25/44] Test get and set on closed stores

---
 src/zarr/storage/_zip.py         |  6 ++++++
 src/zarr/testing/store.py        | 26 ++++++++++++++++++++++++++
 tests/test_store/test_logging.py |  6 ++++--
 tests/test_store/test_wrapper.py |  6 ++++--
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py
index e808b80e4..6715db06f 100644
--- a/src/zarr/storage/_zip.py
+++ b/src/zarr/storage/_zip.py
@@ -146,6 +146,8 @@ def _get(
         prototype: BufferPrototype,
         byte_range: ByteRequest | None = None,
     ) -> Buffer | None:
+        if not self._is_open:
+            self._sync_open()
         # docstring inherited
         try:
             with self._zf.open(key) as f:  # will raise KeyError
@@ -190,6 +192,8 @@ async def get_partial_values(
         return out
 
     def _set(self, key: str, value: Buffer) -> None:
+        if not self._is_open:
+            self._sync_open()
         # generally, this should be called inside a lock
         keyinfo = zipfile.ZipInfo(filename=key, date_time=time.localtime(time.time())[:6])
         keyinfo.compress_type = self.compression
@@ -203,6 +207,8 @@ def _set(self, key: str, value: Buffer) -> None:
     async def set(self, key: str, value: Buffer) -> None:
         # docstring inherited
         self._check_writable()
+        if not self._is_open:
+            self._sync_open()
         assert isinstance(key, str)
         if not isinstance(value, Buffer):
             raise TypeError("ZipStore.set(): `value` must a Buffer instance")
diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 2c27d401d..6d41494ed 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -66,6 +66,10 @@ def open_kwargs(self, store_kwargs: dict[str, Any]) -> dict[str, Any]:
     async def store(self, open_kwargs: dict[str, Any]) -> Store:
         return await self.store_cls.open(**open_kwargs)
 
+    @pytest.fixture
+    async def store_not_open(self, store_kwargs: dict[str, Any]) -> Store:
+        return self.store_cls(**store_kwargs)
+
     def test_store_type(self, store: S) -> None:
         assert isinstance(store, Store)
         assert isinstance(store, self.store_cls)
@@ -137,6 +141,17 @@ async def test_get(self, store: S, key: str, data: bytes, byte_range: ByteReques
         expected = data_buf[start:stop]
         assert_bytes_equal(observed, expected)
 
+    async def test_get_not_open(self, store_not_open: S) -> None:
+        """
+        Ensure that data can be read from the store that isn't yet open using the store.get method.
+        """
+        assert not store_not_open._is_open
+        data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
+        key = "c/0"
+        await self.set(store_not_open, key, data_buf)
+        observed = await store_not_open.get(key, prototype=default_buffer_prototype())
+        assert_bytes_equal(observed, data_buf)
+
     async def test_get_raises(self, store: S) -> None:
         """
         Ensure that a ValueError is raise for invalid byte range syntax
@@ -211,6 +226,17 @@ async def test_set(self, store: S, key: str, data: bytes) -> None:
         observed = await self.get(store, key)
         assert_bytes_equal(observed, data_buf)
 
+    async def test_set_not_open(self, store_not_open: S) -> None:
+        """
+        Ensure that data can be written to the store that's not yet open using the store.set method.
+        """
+        assert not store_not_open._is_open
+        data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
+        key = "c/0"
+        await store_not_open.set(key, data_buf)
+        observed = await self.get(store_not_open, key)
+        assert_bytes_equal(observed, data_buf)
+
     async def test_set_many(self, store: S) -> None:
         """
         Test that a dict of key : value pairs can be inserted into the store via the
diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index 9bf0846e3..7e8c45980 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -11,6 +11,8 @@
 from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
+    from _pytest.compat import LEGACY_PATH
+
     from zarr.abc.store import Store
 
 
@@ -28,8 +30,8 @@ async def set(self, store: LoggingStore, key: str, value: Buffer) -> None:
         (store._store.root / key).write_bytes(value.to_bytes())
 
     @pytest.fixture
-    def store_kwargs(self, local_store) -> dict[str, str]:
-        return {"store": local_store, "log_level": "DEBUG"}
+    def store_kwargs(self, tmpdir: LEGACY_PATH) -> dict[str, str]:
+        return {"store": LocalStore(str(tmpdir)), "log_level": "DEBUG"}
 
     @pytest.fixture
     def open_kwargs(self, tmpdir) -> dict[str, str]:
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 00151b668..507a00f69 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -9,6 +9,8 @@
 from zarr.testing.store import StoreTests
 
 if TYPE_CHECKING:
+    from _pytest.compat import LEGACY_PATH
+
     from zarr.abc.store import Store
     from zarr.core.buffer.core import BufferPrototype
 
@@ -27,8 +29,8 @@ async def set(self, store: WrapperStore, key: str, value: Buffer) -> None:
         (store._store.root / key).write_bytes(value.to_bytes())
 
     @pytest.fixture
-    def store_kwargs(self, local_store) -> dict[str, str]:
-        return {"store": local_store}
+    def store_kwargs(self, tmpdir: LEGACY_PATH) -> dict[str, str]:
+        return {"store": LocalStore(str(tmpdir))}
 
     @pytest.fixture
     def open_kwargs(self, tmpdir) -> dict[str, str]:

From 0792fa838cb54f9ba616cff6c82e5f5c0b488255 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 19:58:12 -0500
Subject: [PATCH 26/44] Test using stores in a context manager

---
 src/zarr/testing/store.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 6d41494ed..aacd739a3 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -100,6 +100,15 @@ async def test_store_open_read_only(self, open_kwargs: dict[str, Any], read_only
         assert store._is_open
         assert store.read_only == read_only
 
+    async def test_store_context_manager(self, open_kwargs: dict[str, Any]) -> None:
+        # Test that the context manager closes the store
+        with await self.store_cls.open(**open_kwargs) as store:
+            assert store._is_open
+            # Test trying to open an already open store
+            with pytest.raises(ValueError):
+                await store._open()
+        assert not store._is_open
+
     async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None:
         kwargs = {**open_kwargs, "read_only": True}
         store = await self.store_cls.open(**kwargs)

From dd0de05d6e86a53e90830d1250fa83001bafe67b Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Sun, 12 Jan 2025 20:11:35 -0500
Subject: [PATCH 27/44] Specify abstract methods for StoreTests

---
 src/zarr/testing/store.py | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index aacd739a3..8eebd2de4 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -2,6 +2,7 @@
 
 import asyncio
 import pickle
+from abc import abstractmethod
 from typing import TYPE_CHECKING, Generic, TypeVar
 
 from zarr.storage import WrapperStore
@@ -37,26 +38,41 @@ class StoreTests(Generic[S, B]):
     store_cls: type[S]
     buffer_cls: type[B]
 
+    @abstractmethod
     async def set(self, store: S, key: str, value: Buffer) -> None:
         """
         Insert a value into a storage backend, with a specific key.
         This should not not use any store methods. Bypassing the store methods allows them to be
         tested.
         """
-        raise NotImplementedError
+        ...
 
+    @abstractmethod
     async def get(self, store: S, key: str) -> Buffer:
         """
         Retrieve a value from a storage backend, by key.
         This should not not use any store methods. Bypassing the store methods allows them to be
         tested.
         """
+        ...
 
-        raise NotImplementedError
-
+    @abstractmethod
     @pytest.fixture
     def store_kwargs(self) -> dict[str, Any]:
-        return {"read_only": False}
+        """Kwargs for instantiating a store"""
+        ...
+
+    @abstractmethod
+    def test_store_repr(self, store: S) -> None: ...
+
+    @abstractmethod
+    def test_store_supports_writes(self, store: S) -> None: ...
+
+    @abstractmethod
+    def test_store_supports_partial_writes(self, store: S) -> None: ...
+
+    @abstractmethod
+    def test_store_supports_listing(self, store: S) -> None: ...
 
     @pytest.fixture
     def open_kwargs(self, store_kwargs: dict[str, Any]) -> dict[str, Any]:
@@ -122,18 +138,6 @@ async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None
         with pytest.raises(ValueError):
             await store.delete("foo")
 
-    def test_store_repr(self, store: S) -> None:
-        raise NotImplementedError
-
-    def test_store_supports_writes(self, store: S) -> None:
-        raise NotImplementedError
-
-    def test_store_supports_partial_writes(self, store: S) -> None:
-        raise NotImplementedError
-
-    def test_store_supports_listing(self, store: S) -> None:
-        raise NotImplementedError
-
     @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"])
     @pytest.mark.parametrize("data", [b"\x01\x02\x03\x04", b""])
     @pytest.mark.parametrize(

From 4dba40c8354b8db3763273d4da76a82420f5b57a Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Mon, 13 Jan 2025 08:57:04 -0500
Subject: [PATCH 28/44] Apply suggestions from code review

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
---
 src/zarr/storage/_fsspec.py    | 2 +-
 src/zarr/testing/store.py      | 2 +-
 tests/test_store/test_core.py  | 4 ++--
 tests/test_store/test_local.py | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py
index 0f6bced8f..3d42855c8 100644
--- a/src/zarr/storage/_fsspec.py
+++ b/src/zarr/storage/_fsspec.py
@@ -255,7 +255,7 @@ async def set(
             await self._open()
         self._check_writable()
         if not isinstance(value, Buffer):
-            raise TypeError("FsspecStore.set(): `value` must a Buffer instance")
+            raise TypeError(f"FsspecStore.set(): `value` must a Buffer instance. Got an instance of {type(value)} instead.")
         path = _dereference_path(self.path, key)
         # write data
         if byte_range:
diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 8eebd2de4..585840492 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -262,7 +262,7 @@ async def test_set_many(self, store: S) -> None:
         for k, v in store_dict.items():
             assert (await self.get(store, k)).to_bytes() == v.to_bytes()
 
-    async def test_set_raises(self, store: S) -> None:
+    async def test_set_invalid_buffer(self, store: S) -> None:
         """
         Ensure that set raises a Type or Value Error for invalid buffer arguments.
         """
diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index 806eaf9bb..4c26a0d6c 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -15,7 +15,7 @@
 @pytest.mark.parametrize("zarr_format", [2, 3])
 async def test_contains_group(local_store, write_group: bool, zarr_format: ZarrFormat) -> None:
     """
-    Test contains group method
+     Test that the contains_group method correctly reports the existence of a group.
     """
     root = Group.from_store(store=local_store, zarr_format=zarr_format)
     if write_group:
@@ -28,7 +28,7 @@ async def test_contains_group(local_store, write_group: bool, zarr_format: ZarrF
 @pytest.mark.parametrize("zarr_format", [2, 3])
 async def test_contains_array(local_store, write_array: bool, zarr_format: ZarrFormat) -> None:
     """
-    Test contains group method
+    Test that the contains array method correctly reports the existence of an array.
     """
     root = Group.from_store(store=local_store, zarr_format=zarr_format)
     if write_array:
diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py
index 524f2b378..d05a986be 100644
--- a/tests/test_store/test_local.py
+++ b/tests/test_store/test_local.py
@@ -61,7 +61,7 @@ def test_invalid_root_raises(self):
 
     async def test_get_with_prototype_default(self, store: LocalStore):
         """
-        Ensure that data can be read using the default prototype method.
+        Ensure that data can be read via ``store.get`` if the prototype keyword argument is unspecified, i.e. set to ``None``.
         """
         data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
         key = "c/0"

From 48abe94e41486c530b62df7f42fedfe0a62c8900 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 10:53:40 -0500
Subject: [PATCH 29/44] Lint

---
 src/zarr/storage/_fsspec.py   | 4 +++-
 tests/test_store/test_core.py | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py
index 3d42855c8..1b0801ae6 100644
--- a/src/zarr/storage/_fsspec.py
+++ b/src/zarr/storage/_fsspec.py
@@ -255,7 +255,9 @@ async def set(
             await self._open()
         self._check_writable()
         if not isinstance(value, Buffer):
-            raise TypeError(f"FsspecStore.set(): `value` must a Buffer instance. Got an instance of {type(value)} instead.")
+            raise TypeError(
+                f"FsspecStore.set(): `value` must a Buffer instance. Got an instance of {type(value)} instead."
+            )
         path = _dereference_path(self.path, key)
         # write data
         if byte_range:
diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index 4c26a0d6c..a5de0d2f5 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -15,7 +15,7 @@
 @pytest.mark.parametrize("zarr_format", [2, 3])
 async def test_contains_group(local_store, write_group: bool, zarr_format: ZarrFormat) -> None:
     """
-     Test that the contains_group method correctly reports the existence of a group.
+    Test that the contains_group method correctly reports the existence of a group.
     """
     root = Group.from_store(store=local_store, zarr_format=zarr_format)
     if write_group:

From bf4589de7c6d6c49e6c7c1331f8d26350fec4c40 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 10:55:25 -0500
Subject: [PATCH 30/44] Fix typing for LoggingStore

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
---
 src/zarr/storage/_logging.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py
index d5b3b9327..78d4f3d85 100644
--- a/src/zarr/storage/_logging.py
+++ b/src/zarr/storage/_logging.py
@@ -22,7 +22,7 @@
 T_Store = TypeVar("T_Store", bound=Store)
 
 
-class LoggingStore(WrapperStore[Store]):
+class LoggingStore(WrapperStore[T_Store]):
     """
     Store wrapper that logs all calls to the wrapped store.
 
@@ -45,7 +45,7 @@ class LoggingStore(WrapperStore[Store]):
 
     def __init__(
         self,
-        store: Store,
+        store: T_Store,
         log_level: str = "DEBUG",
         log_handler: logging.Handler | None = None,
     ) -> None:

From 5b3741773462d8b1fc9448cf4132353237eeed33 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 15:15:26 -0500
Subject: [PATCH 31/44] Match specific Errors in tests

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
---
 src/zarr/storage/_fsspec.py |  2 +-
 src/zarr/storage/_local.py  |  4 +++-
 src/zarr/storage/_memory.py |  4 +++-
 src/zarr/storage/_zip.py    |  4 +++-
 src/zarr/testing/store.py   | 19 +++++++++++++------
 5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py
index 1b0801ae6..85517d28c 100644
--- a/src/zarr/storage/_fsspec.py
+++ b/src/zarr/storage/_fsspec.py
@@ -256,7 +256,7 @@ async def set(
         self._check_writable()
         if not isinstance(value, Buffer):
             raise TypeError(
-                f"FsspecStore.set(): `value` must a Buffer instance. Got an instance of {type(value)} instead."
+                f"FsspecStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead."
             )
         path = _dereference_path(self.path, key)
         # write data
diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py
index 5eaa85c59..5948dd3e2 100644
--- a/src/zarr/storage/_local.py
+++ b/src/zarr/storage/_local.py
@@ -169,7 +169,9 @@ async def _set(self, key: str, value: Buffer, exclusive: bool = False) -> None:
         self._check_writable()
         assert isinstance(key, str)
         if not isinstance(value, Buffer):
-            raise TypeError("LocalStore.set(): `value` must a Buffer instance")
+            raise TypeError(
+                f"LocalStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead."
+            )
         path = self.root / key
         await asyncio.to_thread(_put, path, value, start=None, exclusive=exclusive)
 
diff --git a/src/zarr/storage/_memory.py b/src/zarr/storage/_memory.py
index d35ecbe33..c762546a6 100644
--- a/src/zarr/storage/_memory.py
+++ b/src/zarr/storage/_memory.py
@@ -111,7 +111,9 @@ async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None
         await self._ensure_open()
         assert isinstance(key, str)
         if not isinstance(value, Buffer):
-            raise TypeError(f"Expected Buffer. Got {type(value)}.")
+            raise TypeError(
+                f"MemoryStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead."
+            )
 
         if byte_range is not None:
             buf = self._store_dict[key]
diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py
index 6715db06f..a34806aa5 100644
--- a/src/zarr/storage/_zip.py
+++ b/src/zarr/storage/_zip.py
@@ -211,7 +211,9 @@ async def set(self, key: str, value: Buffer) -> None:
             self._sync_open()
         assert isinstance(key, str)
         if not isinstance(value, Buffer):
-            raise TypeError("ZipStore.set(): `value` must a Buffer instance")
+            raise TypeError(
+                f"ZipStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead."
+            )
         with self._lock:
             self._set(key, value)
 
diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
index 585840492..8ea3160c9 100644
--- a/src/zarr/testing/store.py
+++ b/src/zarr/testing/store.py
@@ -121,7 +121,7 @@ async def test_store_context_manager(self, open_kwargs: dict[str, Any]) -> None:
         with await self.store_cls.open(**open_kwargs) as store:
             assert store._is_open
             # Test trying to open an already open store
-            with pytest.raises(ValueError):
+            with pytest.raises(ValueError, match="store is already open"):
                 await store._open()
         assert not store._is_open
 
@@ -131,11 +131,15 @@ async def test_read_only_store_raises(self, open_kwargs: dict[str, Any]) -> None
         assert store.read_only
 
         # set
-        with pytest.raises(ValueError):
+        with pytest.raises(
+            ValueError, match="store was opened in read-only mode and does not support writing"
+        ):
             await store.set("foo", self.buffer_cls.from_bytes(b"bar"))
 
         # delete
-        with pytest.raises(ValueError):
+        with pytest.raises(
+            ValueError, match="store was opened in read-only mode and does not support writing"
+        ):
             await store.delete("foo")
 
     @pytest.mark.parametrize("key", ["c/0", "foo/c/0.0", "foo/0/0"])
@@ -171,7 +175,7 @@ async def test_get_raises(self, store: S) -> None:
         """
         data_buf = self.buffer_cls.from_bytes(b"\x01\x02\x03\x04")
         await self.set(store, "c/0", data_buf)
-        with pytest.raises((ValueError, TypeError)):
+        with pytest.raises((ValueError, TypeError), match=r"Unexpected byte_range, got.*"):
             await store.get("c/0", prototype=default_buffer_prototype(), byte_range=(0, 2))  # type: ignore[arg-type]
 
     async def test_get_many(self, store: S) -> None:
@@ -222,7 +226,7 @@ async def test_getsize_prefix(self, store: S) -> None:
 
     async def test_getsize_raises(self, store: S) -> None:
         """
-        Test the result of store.getsize().
+        Test that getsize() raise a FileNotFoundError if the key doesn't exist.
         """
         with pytest.raises(FileNotFoundError):
             await store.getsize("c/1000")
@@ -266,7 +270,10 @@ async def test_set_invalid_buffer(self, store: S) -> None:
         """
         Ensure that set raises a Type or Value Error for invalid buffer arguments.
         """
-        with pytest.raises((ValueError, TypeError)):
+        with pytest.raises(
+            (ValueError, TypeError),
+            match=r"\S+\.set\(\): `value` must be a Buffer instance. Got an instance of <class 'int'> instead.",
+        ):
             await store.set("c/0", 0)  # type: ignore[arg-type]
 
     @pytest.mark.parametrize(

From 74647ded7f6648aa4f7cfff6aa367f0b8be243c5 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 15:18:28 -0500
Subject: [PATCH 32/44] Add docstring

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
---
 tests/test_store/test_local.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py
index d05a986be..78b1f6e1f 100644
--- a/tests/test_store/test_local.py
+++ b/tests/test_store/test_local.py
@@ -56,6 +56,9 @@ def test_creates_new_directory(self, tmp_path: pathlib.Path):
         zarr.group(store=store)
 
     def test_invalid_root_raises(self):
+        """
+        Test that a TypeError is raised when a non-str/Path type is used for the `root` argument
+        """
         with pytest.raises(TypeError):
             LocalStore(root=0)
 

From c8ebcd098640d04d725ba48526be0746e9d087ae Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 15:22:49 -0500
Subject: [PATCH 33/44] Parametrize tests

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
---
 tests/test_store/test_core.py | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index a5de0d2f5..74f955fec 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -37,15 +37,14 @@ async def test_contains_array(local_store, write_array: bool, zarr_format: ZarrF
     assert await contains_array(store_path, zarr_format=zarr_format) == write_array
 
 
-async def test_contains_invalid_format_raises(local_store) -> None:
+@pytest.mark.parametrize("func", [contains_array, contains_group])
+async def test_contains_invalid_format_raises(local_store, func: callable) -> None:
     """
     Test contains_group and contains_array raise errors for invalid zarr_formats
     """
     store_path = StorePath(local_store)
     with pytest.raises(ValueError):
-        assert await contains_group(store_path, zarr_format="3.0")
-    with pytest.raises(ValueError):
-        assert await contains_array(store_path, zarr_format="3.0")
+        assert await func(store_path, zarr_format="3.0")
 
 
 @pytest.mark.parametrize("path", [None, "", "bar"])
@@ -97,14 +96,13 @@ async def test_make_store_path_store_path(
     assert store_path.read_only == ro
 
 
-async def test_store_path_invalid_mode_raises(tmpdir: LEGACY_PATH) -> None:
+@pytest.mark.parametrize("modes", [(True, "w"), (False, "x")])
+async def test_store_path_invalid_mode_raises(tmpdir: LEGACY_PATH, modes: tuple) -> None:
     """
     Test that ValueErrors are raise for invalid mode.
     """
     with pytest.raises(ValueError):
-        await StorePath.open(LocalStore(str(tmpdir), read_only=True), path=None, mode="w")
-    with pytest.raises(ValueError):
-        await StorePath.open(LocalStore(str(tmpdir), read_only=False), path=None, mode="x")
+        await StorePath.open(LocalStore(str(tmpdir), read_only=modes[0]), path=None, mode=modes[1])
 
 
 async def test_make_store_path_invalid() -> None:

From 1e96600acc744a1ff4bef43fb8e4f313b02f6873 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 15:24:33 -0500
Subject: [PATCH 34/44] Test for contains group/array at multiple heirarchies

Co-authored-by: Davis Bennett <davis.v.bennett@gmail.com>
---
 tests/test_store/test_core.py | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
index 74f955fec..726da06a5 100644
--- a/tests/test_store/test_core.py
+++ b/tests/test_store/test_core.py
@@ -11,29 +11,35 @@
 from zarr.storage._utils import normalize_path
 
 
+@pytest.mark.parametrize("path", ["foo", "foo/bar"])
 @pytest.mark.parametrize("write_group", [True, False])
 @pytest.mark.parametrize("zarr_format", [2, 3])
-async def test_contains_group(local_store, write_group: bool, zarr_format: ZarrFormat) -> None:
+async def test_contains_group(
+    local_store, path: str, write_group: bool, zarr_format: ZarrFormat
+) -> None:
     """
     Test that the contains_group method correctly reports the existence of a group.
     """
     root = Group.from_store(store=local_store, zarr_format=zarr_format)
     if write_group:
-        root.create_group("foo")
-    store_path = StorePath(local_store, path="foo")
+        root.create_group(path)
+    store_path = StorePath(local_store, path=path)
     assert await contains_group(store_path, zarr_format=zarr_format) == write_group
 
 
+@pytest.mark.parametrize("path", ["foo", "foo/bar"])
 @pytest.mark.parametrize("write_array", [True, False])
 @pytest.mark.parametrize("zarr_format", [2, 3])
-async def test_contains_array(local_store, write_array: bool, zarr_format: ZarrFormat) -> None:
+async def test_contains_array(
+    local_store, path: str, write_array: bool, zarr_format: ZarrFormat
+) -> None:
     """
     Test that the contains array method correctly reports the existence of an array.
     """
     root = Group.from_store(store=local_store, zarr_format=zarr_format)
     if write_array:
-        root.create_array("foo", shape=(100,), chunks=(10,), dtype="i4")
-    store_path = StorePath(local_store, path="foo")
+        root.create_array(path, shape=(100,), chunks=(10,), dtype="i4")
+    store_path = StorePath(local_store, path=path)
     assert await contains_array(store_path, zarr_format=zarr_format) == write_array
 
 

From cc14e07597d7eede903f7c5caf665893f4c488df Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Tue, 14 Jan 2025 15:33:03 -0500
Subject: [PATCH 35/44] Update TypeError on GpuMemoryStore

---
 src/zarr/storage/_memory.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_memory.py b/src/zarr/storage/_memory.py
index c762546a6..b37fc8d5c 100644
--- a/src/zarr/storage/_memory.py
+++ b/src/zarr/storage/_memory.py
@@ -233,8 +233,9 @@ async def set(self, key: str, value: Buffer, byte_range: tuple[int, int] | None
         self._check_writable()
         assert isinstance(key, str)
         if not isinstance(value, Buffer):
-            raise TypeError(f"Expected Buffer. Got {type(value)}.")
-
+            raise TypeError(
+                f"GpuMemoryStore.set(): `value` must be a Buffer instance. Got an instance of {type(value)} instead."
+            )
         # Convert to gpu.Buffer
         gpu_value = value if isinstance(value, gpu.Buffer) else gpu.Buffer.from_buffer(value)
         await super().set(key, gpu_value, byte_range=byte_range)

From bf58808a60e25e0bb61a91027356e550692efbaa Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Thu, 16 Jan 2025 10:27:00 -0500
Subject: [PATCH 36/44] Don't implement _is_open setter on wrapped stores

---
 src/zarr/storage/_logging.py     | 3 +--
 src/zarr/storage/_wrapper.py     | 2 +-
 tests/test_store/test_logging.py | 7 +++++++
 tests/test_store/test_wrapper.py | 7 +++++++
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py
index 78d4f3d85..941ca1762 100644
--- a/src/zarr/storage/_logging.py
+++ b/src/zarr/storage/_logging.py
@@ -137,8 +137,7 @@ def _is_open(self) -> bool:
 
     @_is_open.setter
     def _is_open(self, value: bool) -> None:
-        with self.log(value):
-            self._store._is_open = value
+        raise NotImplementedError("LoggingStore must be opened via the `_open` method")
 
     async def _open(self) -> None:
         with self.log():
diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
index 002e1e73c..3bdc1a351 100644
--- a/src/zarr/storage/_wrapper.py
+++ b/src/zarr/storage/_wrapper.py
@@ -62,7 +62,7 @@ def _is_open(self) -> bool:
 
     @_is_open.setter
     def _is_open(self, value: bool) -> None:
-        self._store._is_open = value
+        raise NotImplementedError("WrapperStore must be opened via the `_open` method")
 
     async def clear(self) -> None:
         return await self._store.clear()
diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index 7e8c45980..840a246dc 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -71,6 +71,13 @@ async def test_default_handler(self, local_store, capsys) -> None:
         for h in handlers:
             logging.getLogger().addHandler(h)
 
+    def test_is_open_setter_raises(self, store: LoggingStore) -> None:
+        "Test that a user cannot change `_is_open` without opening the underlying store."
+        with pytest.raises(
+            NotImplementedError, match="LoggingStore must be opened via the `_open` method"
+        ):
+            store._is_open = True
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=["store"])
 async def test_logging_store(store: Store, caplog) -> None:
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 507a00f69..9d34a0701 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -52,6 +52,13 @@ def test_store_supports_listing(self, store: WrapperStore) -> None:
     def test_store_repr(self, store: WrapperStore) -> None:
         assert str(store) == f"wrapping-file://{store._store.root.as_posix()}"
 
+    def test_is_open_setter_raises(self, store: WrapperStore) -> None:
+        "Test that a user cannot change `_is_open` without opening the underlying store."
+        with pytest.raises(
+            NotImplementedError, match="WrapperStore must be opened via the `_open` method"
+        ):
+            store._is_open = True
+
 
 @pytest.mark.parametrize("store", ["local", "memory", "zip"], indirect=True)
 async def test_wrapped_set(store: Store, capsys: pytest.CaptureFixture[str]) -> None:

From e1caef0df9738b2df6c9f6fcdb7211f72b409acd Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Thu, 16 Jan 2025 10:49:44 -0500
Subject: [PATCH 37/44] Update reprs for LoggingStore and WrapperStore

---
 src/zarr/storage/_logging.py     | 2 +-
 src/zarr/storage/_wrapper.py     | 2 +-
 tests/test_store/test_logging.py | 3 +++
 tests/test_store/test_wrapper.py | 3 +++
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_logging.py b/src/zarr/storage/_logging.py
index 941ca1762..e9d621158 100644
--- a/src/zarr/storage/_logging.py
+++ b/src/zarr/storage/_logging.py
@@ -161,7 +161,7 @@ def __str__(self) -> str:
         return f"logging-{self._store}"
 
     def __repr__(self) -> str:
-        return f"LoggingStore({repr(self._store)!r})"
+        return f"LoggingStore({self._store.__class__.__name__}, '{self._store}')"
 
     def __eq__(self, other: object) -> bool:
         with self.log(other):
diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
index 3bdc1a351..349048e49 100644
--- a/src/zarr/storage/_wrapper.py
+++ b/src/zarr/storage/_wrapper.py
@@ -81,7 +81,7 @@ def __str__(self) -> str:
         return f"wrapping-{self._store}"
 
     def __repr__(self) -> str:
-        return f"WrapperStore({repr(self._store)!r})"
+        return f"WrapperStore({self._store.__class__.__name__}, '{self._store}')"
 
     async def get(
         self, key: str, prototype: BufferPrototype, byte_range: ByteRequest | None = None
diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
index 840a246dc..1a89dca87 100644
--- a/tests/test_store/test_logging.py
+++ b/tests/test_store/test_logging.py
@@ -51,6 +51,9 @@ def test_store_supports_listing(self, store: LoggingStore) -> None:
         assert store.supports_listing
 
     def test_store_repr(self, store: LoggingStore) -> None:
+        assert f"{store!r}" == f"LoggingStore(LocalStore, 'file://{store._store.root.as_posix()}')"
+
+    def test_store_str(self, store: LoggingStore) -> None:
         assert str(store) == f"logging-file://{store._store.root.as_posix()}"
 
     async def test_default_handler(self, local_store, capsys) -> None:
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index 9d34a0701..d04d07644 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -50,6 +50,9 @@ def test_store_supports_listing(self, store: WrapperStore) -> None:
         assert store.supports_listing
 
     def test_store_repr(self, store: WrapperStore) -> None:
+        assert f"{store!r}" == f"WrapperStore(LocalStore, 'file://{store._store.root.as_posix()}')"
+
+    def test_store_str(self, store: WrapperStore) -> None:
         assert str(store) == f"wrapping-file://{store._store.root.as_posix()}"
 
     def test_is_open_setter_raises(self, store: WrapperStore) -> None:

From 1922d2da3678d0f8e4cf3c24c8124db010aacaef Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Thu, 16 Jan 2025 11:05:13 -0500
Subject: [PATCH 38/44] Test check_writeable and close for WrapperStore

---
 tests/test_store/test_wrapper.py | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py
index d04d07644..7e933548b 100644
--- a/tests/test_store/test_wrapper.py
+++ b/tests/test_store/test_wrapper.py
@@ -36,10 +36,6 @@ def store_kwargs(self, tmpdir: LEGACY_PATH) -> dict[str, str]:
     def open_kwargs(self, tmpdir) -> dict[str, str]:
         return {"store_cls": LocalStore, "root": str(tmpdir)}
 
-    @pytest.fixture
-    def store(self, store_kwargs: dict[str, str]) -> WrapperStore:
-        return self.store_cls(**store_kwargs)
-
     def test_store_supports_writes(self, store: WrapperStore) -> None:
         assert store.supports_writes
 
@@ -55,8 +51,21 @@ def test_store_repr(self, store: WrapperStore) -> None:
     def test_store_str(self, store: WrapperStore) -> None:
         assert str(store) == f"wrapping-file://{store._store.root.as_posix()}"
 
+    def test_check_writeable(self, store: WrapperStore) -> None:
+        """
+        Test _check_writeable() runs without errors.
+        """
+        store._check_writable()
+
+    def test_close(self, store: WrapperStore) -> None:
+        "Test store can be closed"
+        store.close()
+        assert not store._is_open
+
     def test_is_open_setter_raises(self, store: WrapperStore) -> None:
-        "Test that a user cannot change `_is_open` without opening the underlying store."
+        """
+        Test that a user cannot change `_is_open` without opening the underlying store.
+        """
         with pytest.raises(
             NotImplementedError, match="WrapperStore must be opened via the `_open` method"
         ):

From 45ea40d1ef4f8dd5b15247e956673a45d7ba1e9a Mon Sep 17 00:00:00 2001
From: Hannes Spitz <44113112+brokkoli71@users.noreply.github.com>
Date: Thu, 16 Jan 2025 12:05:49 +0100
Subject: [PATCH 39/44] Update pull request template (#2717)

---
 .github/PULL_REQUEST_TEMPLATE.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index a0d41f984..723c995ce 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -3,7 +3,7 @@
 TODO:
 * [ ] Add unit tests and/or doctests in docstrings
 * [ ] Add docstrings and API docs for any new/modified user-facing classes and functions
-* [ ] New/modified features documented in docs/tutorial.rst
-* [ ] Changes documented in docs/release.rst
+* [ ] New/modified features documented in `docs/user-guide/*.rst`
+* [ ] Changes documented in `docs/release-notes.rst`
 * [ ] GitHub Actions have all passed
 * [ ] Test coverage is 100% (Codecov passes)

From b281bc95db04521cf2bdebbe82d8231f71d52dd8 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Thu, 16 Jan 2025 11:10:04 -0500
Subject: [PATCH 40/44] Add release notes

---
 docs/release-notes.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/release-notes.rst b/docs/release-notes.rst
index ecd413510..9d174aa77 100644
--- a/docs/release-notes.rst
+++ b/docs/release-notes.rst
@@ -13,7 +13,9 @@ Bug fixes
 
 * Fixes a bug that prevented reading Zarr format 2 data with consolidated metadata written using ``zarr-python`` version 2 (:issue:`2694`).
 
-* Ensure that compressor=None results in no compression when writing Zarr format 2 data (:issue:`2708`)
+* Ensure that compressor=None results in no compression when writing Zarr format 2 data (:issue:`2708`).
+
+* Improves consistency of reprs, open/close behavior, and equality checking across Stores (:issue:`2693`).
 
 Behaviour changes
 ~~~~~~~~~~~~~~~~~

From 0d6eccfba8c13a894ccb78b054b6c40644b154c2 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Thu, 23 Jan 2025 15:30:12 -0500
Subject: [PATCH 41/44] Comprehensive changelog entry

---
 changes/2693.bugfix.rst | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 changes/2693.bugfix.rst

diff --git a/changes/2693.bugfix.rst b/changes/2693.bugfix.rst
new file mode 100644
index 000000000..7f0e56408
--- /dev/null
+++ b/changes/2693.bugfix.rst
@@ -0,0 +1,14 @@
+Implement open() for LoggingStore
+LoggingStore is now a generic class.
+Add _is_open property and setter for WrapperStore
+Use stdout rather than stderr as the default stream for LoggingStore
+Ensure that ZipStore is open before getting or setting any values
+Update equality for LoggingStore and WrapperStore such that the types must be equal.
+Indicate StoreTest's `test_store_repr`, `test_store_supports_writes`, `test_store_supports_partial_writes`, and `test_store_supports_listing` need to be implemented using `@abstractmethod` rather than `NotImplementedError`.
+Separate instantiating and opening a store in StoreTests
+Test using Store as a context manager in StoreTests
+Match the errors raised by read only stores in StoreTests
+Test that a ValueError is raise for invalid byte range syntax in StoreTests
+Test getsize() and getsize_prefix() in StoreTests
+Test the error raise for invalid buffer arguments in StoreTests
+Test that data can be written to a store that's not yet open using the store.set method in StoreTests
\ No newline at end of file

From 905913504722a9e6a6ca2ad1eb8a6f54f6e6d2d3 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Thu, 23 Jan 2025 15:37:36 -0500
Subject: [PATCH 42/44] Match error message

---
 src/zarr/storage/_local.py     | 2 +-
 tests/test_store/test_local.py | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/zarr/storage/_local.py b/src/zarr/storage/_local.py
index 5948dd3e2..1defea26b 100644
--- a/src/zarr/storage/_local.py
+++ b/src/zarr/storage/_local.py
@@ -96,7 +96,7 @@ def __init__(self, root: Path | str, *, read_only: bool = False) -> None:
             root = Path(root)
         if not isinstance(root, Path):
             raise TypeError(
-                f'"root" must be a string or Path instance. Got an object with type {type(root)} instead.'
+                f"'root' must be a string or Path instance. Got an instance of {type(root)} instead."
             )
         self.root = root
 
diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py
index 78b1f6e1f..d9d941c6f 100644
--- a/tests/test_store/test_local.py
+++ b/tests/test_store/test_local.py
@@ -59,7 +59,10 @@ def test_invalid_root_raises(self):
         """
         Test that a TypeError is raised when a non-str/Path type is used for the `root` argument
         """
-        with pytest.raises(TypeError):
+        with pytest.raises(
+            TypeError,
+            match=r"'root' must be a string or Path instance. Got an instance of <class 'int'> instead.",
+        ):
             LocalStore(root=0)
 
     async def test_get_with_prototype_default(self, store: LocalStore):

From 1587fd1e5f31d501da8feb7f4a0be14f263a93b7 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Fri, 24 Jan 2025 11:40:59 -0500
Subject: [PATCH 43/44] Apply suggestions from code review

Co-authored-by: David Stansby <dstansby@gmail.com>
---
 changes/2693.bugfix.rst | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/changes/2693.bugfix.rst b/changes/2693.bugfix.rst
index 7f0e56408..46009358e 100644
--- a/changes/2693.bugfix.rst
+++ b/changes/2693.bugfix.rst
@@ -1,6 +1,5 @@
 Implement open() for LoggingStore
 LoggingStore is now a generic class.
-Add _is_open property and setter for WrapperStore
 Use stdout rather than stderr as the default stream for LoggingStore
 Ensure that ZipStore is open before getting or setting any values
 Update equality for LoggingStore and WrapperStore such that the types must be equal.
@@ -10,5 +9,5 @@ Test using Store as a context manager in StoreTests
 Match the errors raised by read only stores in StoreTests
 Test that a ValueError is raise for invalid byte range syntax in StoreTests
 Test getsize() and getsize_prefix() in StoreTests
-Test the error raise for invalid buffer arguments in StoreTests
+Test the error raised for invalid buffer arguments in StoreTests
 Test that data can be written to a store that's not yet open using the store.set method in StoreTests
\ No newline at end of file

From 5e16b3d66a2b1ae091f27449c6b1d698d2ffc570 Mon Sep 17 00:00:00 2001
From: Max Jones <14077947+maxrjones@users.noreply.github.com>
Date: Fri, 24 Jan 2025 13:46:41 -0500
Subject: [PATCH 44/44] Update 2693.bugfix.rst

---
 changes/2693.bugfix.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/changes/2693.bugfix.rst b/changes/2693.bugfix.rst
index 46009358e..14b45a221 100644
--- a/changes/2693.bugfix.rst
+++ b/changes/2693.bugfix.rst
@@ -2,7 +2,7 @@ Implement open() for LoggingStore
 LoggingStore is now a generic class.
 Use stdout rather than stderr as the default stream for LoggingStore
 Ensure that ZipStore is open before getting or setting any values
-Update equality for LoggingStore and WrapperStore such that the types must be equal.
+Update equality for LoggingStore and WrapperStore such that 'other' must also be a LoggingStore or WrapperStore respectively, rather than only checking the types of the stores they wrap.
 Indicate StoreTest's `test_store_repr`, `test_store_supports_writes`, `test_store_supports_partial_writes`, and `test_store_supports_listing` need to be implemented using `@abstractmethod` rather than `NotImplementedError`.
 Separate instantiating and opening a store in StoreTests
 Test using Store as a context manager in StoreTests
@@ -10,4 +10,4 @@ Match the errors raised by read only stores in StoreTests
 Test that a ValueError is raise for invalid byte range syntax in StoreTests
 Test getsize() and getsize_prefix() in StoreTests
 Test the error raised for invalid buffer arguments in StoreTests
-Test that data can be written to a store that's not yet open using the store.set method in StoreTests
\ No newline at end of file
+Test that data can be written to a store that's not yet open using the store.set method in StoreTests