From 3c8be19a166b3c90803463a34e6a61dcbf60243f Mon Sep 17 00:00:00 2001 From: David Stansby Date: Sat, 10 May 2025 10:28:21 +0100 Subject: [PATCH 1/6] Ignore explicit test store files --- .pre-commit-config.yaml | 1 + pyproject.toml | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 474d109c80..80743a5dec 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,6 +38,7 @@ repos: # Tests - pytest - hypothesis + - s3fs - repo: https://github.com/scientific-python/cookie rev: 2025.01.22 hooks: diff --git a/pyproject.toml b/pyproject.toml index 1c534f7927..c06e527cde 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -368,7 +368,15 @@ strict = false module = [ "tests.test_codecs.test_codecs", "tests.test_metadata.*", - "tests.test_store.*", + "tests.test_store.test_core", + "tests.test_store.test_fsspec", + "tests.test_store.test_local", + "tests.test_store.test_logging", + "tests.test_store.test_memory", + "tests.test_store.test_object", + "tests.test_store.test_stateful", + "tests.test_store.test_wrapper", + "tests.test_store.test_zip", "tests.test_group", "tests.test_indexing", "tests.test_properties", From 792f0308d90fda0bfdc0db484b297e62bf9f96e5 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Sat, 10 May 2025 10:32:59 +0100 Subject: [PATCH 2/6] Fix test_zip --- pyproject.toml | 4 ++-- tests/test_store/test_zip.py | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c06e527cde..5c78bfe729 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -358,7 +358,8 @@ module = [ "tests.package_with_entrypoint.*", "zarr.testing.stateful", "tests.test_codecs.test_transpose", - "tests.test_config" + "tests.test_config", + "tests.test_store.test_zip" ] strict = false @@ -376,7 +377,6 @@ module = [ "tests.test_store.test_object", "tests.test_store.test_stateful", "tests.test_store.test_wrapper", - "tests.test_store.test_zip", "tests.test_group", "tests.test_indexing", "tests.test_properties", diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index 0237258ab1..fa99ca61bd 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -11,6 +11,7 @@ import zarr from zarr.core.buffer import Buffer, cpu, default_buffer_prototype +from zarr.core.group import Group from zarr.storage import ZipStore from zarr.testing.store import StoreTests @@ -32,7 +33,7 @@ class TestZipStore(StoreTests[ZipStore, cpu.Buffer]): buffer_cls = cpu.Buffer @pytest.fixture - def store_kwargs(self, request) -> dict[str, str | bool]: + def store_kwargs(self) -> dict[str, str | bool]: fd, temp_path = tempfile.mkstemp() os.close(fd) os.unlink(temp_path) @@ -40,12 +41,14 @@ def store_kwargs(self, request) -> dict[str, str | bool]: return {"path": temp_path, "mode": "w", "read_only": False} async def get(self, store: ZipStore, key: str) -> Buffer: - return store._get(key, prototype=default_buffer_prototype()) + buf = store._get(key, prototype=default_buffer_prototype()) + assert buf is not None + return buf async def set(self, store: ZipStore, key: str, value: Buffer) -> None: return store._set(key, value) - def test_store_read_only(self, store: ZipStore, store_kwargs: dict[str, Any]) -> None: + def test_store_read_only(self, store: ZipStore) -> None: assert not store.read_only async def test_read_only_store_raises(self, store_kwargs: dict[str, Any]) -> None: @@ -109,7 +112,7 @@ def test_api_integration(self, store: ZipStore) -> None: async def test_store_open_read_only( self, store_kwargs: dict[str, Any], read_only: bool ) -> None: - if read_only == "r": + if read_only: # create an empty zipfile with zipfile.ZipFile(store_kwargs["path"], mode="w"): pass @@ -129,9 +132,11 @@ def test_externally_zipped_store(self, tmp_path: Path) -> None: zarr_path = tmp_path / "foo.zarr" root = zarr.open_group(store=zarr_path, mode="w") root.require_group("foo") - root["foo"]["bar"] = np.array([1]) - shutil.make_archive(zarr_path, "zip", zarr_path) + assert isinstance(foo := root["foo"], Group) # noqa: RUF018 + foo["bar"] = np.array([1]) + shutil.make_archive(str(zarr_path), "zip", zarr_path) zip_path = tmp_path / "foo.zarr.zip" zipped = zarr.open_group(ZipStore(zip_path, mode="r"), mode="r") assert list(zipped.keys()) == list(root.keys()) - assert list(zipped["foo"].keys()) == list(root["foo"].keys()) + assert isinstance(group := zipped["foo"], Group) + assert list(group.keys()) == list(group.keys()) From a6bd0a2dab7f14749bd637b1fce0f083da942310 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Sat, 10 May 2025 10:46:31 +0100 Subject: [PATCH 3/6] Fix test_local --- pyproject.toml | 4 ++-- src/zarr/testing/store.py | 2 +- tests/test_store/test_local.py | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5c78bfe729..ba34b86eb0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -359,7 +359,8 @@ module = [ "zarr.testing.stateful", "tests.test_codecs.test_transpose", "tests.test_config", - "tests.test_store.test_zip" + "tests.test_store.test_zip", + "tests.test_store.test_local", ] strict = false @@ -371,7 +372,6 @@ module = [ "tests.test_metadata.*", "tests.test_store.test_core", "tests.test_store.test_fsspec", - "tests.test_store.test_local", "tests.test_store.test_logging", "tests.test_store.test_memory", "tests.test_store.test_object", diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 867df2121f..0e73599791 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -58,7 +58,7 @@ async def get(self, store: S, key: str) -> Buffer: @abstractmethod @pytest.fixture - def store_kwargs(self) -> dict[str, Any]: + def store_kwargs(self, *args: Any, **kwargs: Any) -> dict[str, Any]: """Kwargs for instantiating a store""" ... diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py index d9d941c6f0..8699a85082 100644 --- a/tests/test_store/test_local.py +++ b/tests/test_store/test_local.py @@ -28,7 +28,7 @@ async def set(self, store: LocalStore, key: str, value: Buffer) -> None: (store.root / key).write_bytes(value.to_bytes()) @pytest.fixture - def store_kwargs(self, tmpdir) -> dict[str, str]: + def store_kwargs(self, tmpdir: str) -> dict[str, str]: return {"root": str(tmpdir)} def test_store_repr(self, store: LocalStore) -> None: @@ -48,14 +48,14 @@ async def test_empty_with_empty_subdir(self, store: LocalStore) -> None: (store.root / "foo/bar").mkdir(parents=True) assert await store.is_empty("") - def test_creates_new_directory(self, tmp_path: pathlib.Path): + def test_creates_new_directory(self, tmp_path: pathlib.Path) -> None: target = tmp_path.joinpath("a", "b", "c") assert not target.exists() store = self.store_cls(root=target) zarr.group(store=store) - def test_invalid_root_raises(self): + def test_invalid_root_raises(self) -> None: """ Test that a TypeError is raised when a non-str/Path type is used for the `root` argument """ @@ -63,9 +63,9 @@ def test_invalid_root_raises(self): TypeError, match=r"'root' must be a string or Path instance. Got an instance of instead.", ): - LocalStore(root=0) + LocalStore(root=0) # type: ignore[arg-type] - async def test_get_with_prototype_default(self, store: LocalStore): + async def test_get_with_prototype_default(self, store: LocalStore) -> None: """ Ensure that data can be read via ``store.get`` if the prototype keyword argument is unspecified, i.e. set to ``None``. """ From 3c750ad15c7a37aa609456237cd243ac1c13235e Mon Sep 17 00:00:00 2001 From: David Stansby Date: Sat, 10 May 2025 10:53:07 +0100 Subject: [PATCH 4/6] Fix test_fsspec --- pyproject.toml | 2 +- tests/test_store/test_fsspec.py | 37 +++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ba34b86eb0..800c47933a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -361,6 +361,7 @@ module = [ "tests.test_config", "tests.test_store.test_zip", "tests.test_store.test_local", + "tests.test_store.test_fsspec" ] strict = false @@ -371,7 +372,6 @@ module = [ "tests.test_codecs.test_codecs", "tests.test_metadata.*", "tests.test_store.test_core", - "tests.test_store.test_fsspec", "tests.test_store.test_logging", "tests.test_store.test_memory", "tests.test_store.test_object", diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py index 08cf2f286d..f09f8173e5 100644 --- a/tests/test_store/test_fsspec.py +++ b/tests/test_store/test_fsspec.py @@ -3,7 +3,7 @@ import json import os import re -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any import pytest from packaging.version import parse as parse_version @@ -17,8 +17,13 @@ if TYPE_CHECKING: from collections.abc import Generator + from pathlib import Path import botocore.client + import s3fs + + from zarr.core.common import JSON + # Warning filter due to https://github.com/boto/boto3/issues/3889 pytestmark = [ @@ -109,10 +114,12 @@ async def test_basic() -> None: data = b"hello" await store.set("foo", cpu.Buffer.from_bytes(data)) assert await store.exists("foo") - assert (await store.get("foo", prototype=default_buffer_prototype())).to_bytes() == data + assert (buf := (await store.get("foo", prototype=default_buffer_prototype()))) is not None + assert buf.to_bytes() == data out = await store.get_partial_values( prototype=default_buffer_prototype(), key_ranges=[("foo", OffsetByteRequest(1))] ) + assert out[0] is not None assert out[0].to_bytes() == data[1:] @@ -121,7 +128,7 @@ class TestFsspecStoreS3(StoreTests[FsspecStore, cpu.Buffer]): buffer_cls = cpu.Buffer @pytest.fixture - def store_kwargs(self, request) -> dict[str, str | bool]: + def store_kwargs(self) -> dict[str, str | bool]: try: from fsspec import url_to_fs except ImportError: @@ -133,7 +140,7 @@ def store_kwargs(self, request) -> dict[str, str | bool]: return {"fs": fs, "path": path} @pytest.fixture - def store(self, store_kwargs: dict[str, str | bool]) -> FsspecStore: + async def store(self, store_kwargs: dict[str, Any]) -> FsspecStore: return self.store_cls(**store_kwargs) async def get(self, store: FsspecStore, key: str) -> Buffer: @@ -168,7 +175,11 @@ async def test_fsspec_store_from_uri(self, store: FsspecStore) -> None: "anon": False, } - meta = {"attributes": {"key": "value"}, "zarr_format": 3, "node_type": "group"} + meta: dict[str, JSON] = { + "attributes": {"key": "value"}, + "zarr_format": 3, + "node_type": "group", + } await store.set( "zarr.json", @@ -179,7 +190,7 @@ async def test_fsspec_store_from_uri(self, store: FsspecStore) -> None: ) assert dict(group.attrs) == {"key": "value"} - meta["attributes"]["key"] = "value-2" + meta["attributes"]["key"] = "value-2" # type: ignore[index] await store.set( "directory-2/zarr.json", self.buffer_cls.from_bytes(json.dumps(meta).encode()), @@ -189,7 +200,7 @@ async def test_fsspec_store_from_uri(self, store: FsspecStore) -> None: ) assert dict(group.attrs) == {"key": "value-2"} - meta["attributes"]["key"] = "value-3" + meta["attributes"]["key"] = "value-3" # type: ignore[index] await store.set( "directory-3/zarr.json", self.buffer_cls.from_bytes(json.dumps(meta).encode()), @@ -216,7 +227,7 @@ def test_from_upath(self) -> None: assert result.fs.asynchronous assert result.path == f"{test_bucket_name}/foo/bar" - def test_init_raises_if_path_has_scheme(self, store_kwargs) -> None: + def test_init_raises_if_path_has_scheme(self, store_kwargs: dict[str, Any]) -> None: # regression test for https://github.com/zarr-developers/zarr-python/issues/2342 store_kwargs["path"] = "s3://" + store_kwargs["path"] with pytest.raises( @@ -237,7 +248,7 @@ def test_init_warns_if_fs_asynchronous_is_false(self) -> None: with pytest.warns(UserWarning, match=r".* was not created with `asynchronous=True`.*"): self.store_cls(**store_kwargs) - async def test_empty_nonexistent_path(self, store_kwargs) -> None: + async def test_empty_nonexistent_path(self, store_kwargs: dict[str, Any]) -> None: # regression test for https://github.com/zarr-developers/zarr-python/pull/2343 store_kwargs["path"] += "/abc" store = await self.store_cls.open(**store_kwargs) @@ -256,7 +267,7 @@ async def test_delete_dir_unsupported_deletes(self, store: FsspecStore) -> None: parse_version(fsspec.__version__) < parse_version("2024.12.0"), reason="No AsyncFileSystemWrapper", ) -def test_wrap_sync_filesystem(): +def test_wrap_sync_filesystem() -> None: """The local fs is not async so we should expect it to be wrapped automatically""" from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper @@ -270,7 +281,7 @@ def test_wrap_sync_filesystem(): parse_version(fsspec.__version__) < parse_version("2024.12.0"), reason="No AsyncFileSystemWrapper", ) -def test_no_wrap_async_filesystem(): +def test_no_wrap_async_filesystem() -> None: """An async fs should not be wrapped automatically; fsspec's https filesystem is such an fs""" from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper @@ -284,12 +295,12 @@ def test_no_wrap_async_filesystem(): parse_version(fsspec.__version__) < parse_version("2024.12.0"), reason="No AsyncFileSystemWrapper", ) -async def test_delete_dir_wrapped_filesystem(tmpdir) -> None: +async def test_delete_dir_wrapped_filesystem(tmp_path: Path) -> None: from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper from fsspec.implementations.local import LocalFileSystem wrapped_fs = AsyncFileSystemWrapper(LocalFileSystem(auto_mkdir=True)) - store = FsspecStore(wrapped_fs, read_only=False, path=f"{tmpdir}/test/path") + store = FsspecStore(wrapped_fs, read_only=False, path=f"{tmp_path}/test/path") assert isinstance(store.fs, AsyncFileSystemWrapper) assert store.fs.asynchronous From 3e220e795c572a7ba7caefb9e96b5dd982ee14e4 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Sat, 10 May 2025 11:05:04 +0100 Subject: [PATCH 5/6] Fix typing in test_memory --- pyproject.toml | 4 ++-- src/zarr/testing/utils.py | 9 ++++---- tests/test_store/test_memory.py | 41 ++++++++++++++++++--------------- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 800c47933a..033c9dc114 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -361,7 +361,8 @@ module = [ "tests.test_config", "tests.test_store.test_zip", "tests.test_store.test_local", - "tests.test_store.test_fsspec" + "tests.test_store.test_fsspec", + "tests.test_store.test_memory", ] strict = false @@ -373,7 +374,6 @@ module = [ "tests.test_metadata.*", "tests.test_store.test_core", "tests.test_store.test_logging", - "tests.test_store.test_memory", "tests.test_store.test_object", "tests.test_store.test_stateful", "tests.test_store.test_wrapper", diff --git a/src/zarr/testing/utils.py b/src/zarr/testing/utils.py index 28d6774286..7cf57ab9d6 100644 --- a/src/zarr/testing/utils.py +++ b/src/zarr/testing/utils.py @@ -1,7 +1,6 @@ from __future__ import annotations -from collections.abc import Callable, Coroutine -from typing import TYPE_CHECKING, Any, TypeVar, cast +from typing import TYPE_CHECKING, TypeVar, cast import pytest @@ -38,13 +37,13 @@ def has_cupy() -> bool: return False -T_Callable = TypeVar("T_Callable", bound=Callable[..., Coroutine[Any, Any, None] | None]) +T = TypeVar("T") # Decorator for GPU tests -def gpu_test(func: T_Callable) -> T_Callable: +def gpu_test(func: T) -> T: return cast( - "T_Callable", + "T", pytest.mark.gpu( pytest.mark.skipif(not has_cupy(), reason="CuPy not installed or no GPU available")( func diff --git a/tests/test_store/test_memory.py b/tests/test_store/test_memory.py index e520c7d054..a090f56951 100644 --- a/tests/test_store/test_memory.py +++ b/tests/test_store/test_memory.py @@ -1,12 +1,15 @@ from __future__ import annotations import re -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any import numpy as np +import numpy.typing as npt import pytest import zarr +import zarr.core +import zarr.core.array from zarr.core.buffer import Buffer, cpu, gpu from zarr.storage import GpuMemoryStore, MemoryStore from zarr.testing.store import StoreTests @@ -31,16 +34,16 @@ async def get(self, store: MemoryStore, key: str) -> Buffer: return store._store_dict[key] @pytest.fixture(params=[None, True]) - def store_kwargs( - self, request: pytest.FixtureRequest - ) -> dict[str, str | dict[str, Buffer] | None]: - kwargs = {"store_dict": None} + def store_kwargs(self, request: pytest.FixtureRequest) -> dict[str, Any]: + kwargs: dict[str, Any] if request.param is True: - kwargs["store_dict"] = {} + kwargs = {"store_dict": {}} + else: + kwargs = {"store_dict": None} return kwargs @pytest.fixture - def store(self, store_kwargs: str | dict[str, Buffer] | None) -> MemoryStore: + async def store(self, store_kwargs: dict[str, Any]) -> MemoryStore: return self.store_cls(**store_kwargs) def test_store_repr(self, store: MemoryStore) -> None: @@ -55,13 +58,13 @@ def test_store_supports_listing(self, store: MemoryStore) -> None: def test_store_supports_partial_writes(self, store: MemoryStore) -> None: assert store.supports_partial_writes - def test_list_prefix(self, store: MemoryStore) -> None: + async def test_list_prefix(self, store: MemoryStore) -> None: assert True @pytest.mark.parametrize("dtype", ["uint8", "float32", "int64"]) @pytest.mark.parametrize("zarr_format", [2, 3]) async def test_deterministic_size( - self, store: MemoryStore, dtype, zarr_format: ZarrFormat + self, store: MemoryStore, dtype: npt.DTypeLike, zarr_format: ZarrFormat ) -> None: a = zarr.empty( store=store, @@ -85,23 +88,23 @@ class TestGpuMemoryStore(StoreTests[GpuMemoryStore, gpu.Buffer]): store_cls = GpuMemoryStore buffer_cls = gpu.Buffer - async def set(self, store: GpuMemoryStore, key: str, value: Buffer) -> None: + async def set(self, store: GpuMemoryStore, key: str, value: gpu.Buffer) -> None: # type: ignore[override] store._store_dict[key] = value async def get(self, store: MemoryStore, key: str) -> Buffer: return store._store_dict[key] @pytest.fixture(params=[None, True]) - def store_kwargs( - self, request: pytest.FixtureRequest - ) -> dict[str, str | dict[str, Buffer] | None]: - kwargs = {"store_dict": None} + def store_kwargs(self, request: pytest.FixtureRequest) -> dict[str, Any]: + kwargs: dict[str, Any] if request.param is True: - kwargs["store_dict"] = {} + kwargs = {"store_dict": {}} + else: + kwargs = {"store_dict": None} return kwargs @pytest.fixture - def store(self, store_kwargs: str | dict[str, gpu.Buffer] | None) -> GpuMemoryStore: + async def store(self, store_kwargs: dict[str, Any]) -> GpuMemoryStore: return self.store_cls(**store_kwargs) def test_store_repr(self, store: GpuMemoryStore) -> None: @@ -116,15 +119,15 @@ def test_store_supports_listing(self, store: GpuMemoryStore) -> None: def test_store_supports_partial_writes(self, store: GpuMemoryStore) -> None: assert store.supports_partial_writes - def test_list_prefix(self, store: GpuMemoryStore) -> None: + async def test_list_prefix(self, store: GpuMemoryStore) -> None: assert True def test_dict_reference(self, store: GpuMemoryStore) -> None: - store_dict = {} + store_dict: dict[str, Any] = {} result = GpuMemoryStore(store_dict=store_dict) assert result._store_dict is store_dict - def test_from_dict(self): + def test_from_dict(self) -> None: d = { "a": gpu.Buffer.from_bytes(b"aaaa"), "b": cpu.Buffer.from_bytes(b"bbbb"), From ee9dd9fe8860fdb42df311e5cfa5e62fe4ee33f7 Mon Sep 17 00:00:00 2001 From: David Stansby Date: Tue, 13 May 2025 11:33:01 +0100 Subject: [PATCH 6/6] Remove walrus Co-authored-by: Davis Bennett --- tests/test_store/test_fsspec.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py index f09f8173e5..c10471809c 100644 --- a/tests/test_store/test_fsspec.py +++ b/tests/test_store/test_fsspec.py @@ -114,7 +114,8 @@ async def test_basic() -> None: data = b"hello" await store.set("foo", cpu.Buffer.from_bytes(data)) assert await store.exists("foo") - assert (buf := (await store.get("foo", prototype=default_buffer_prototype()))) is not None + buf = await store.get("foo", prototype=default_buffer_prototype()) + assert buf is not None assert buf.to_bytes() == data out = await store.get_partial_values( prototype=default_buffer_prototype(), key_ranges=[("foo", OffsetByteRequest(1))]