Merge branch 'main' of https://github.com/zarr-developers/zarr-python into feature/store_erase_prefix

Author: jhamman

.github/workflows/test.yml

@@ -43,3 +43,35 @@ jobs:
     - name: Run Tests
       run: |
         hatch env run --env test.py${{ matrix.python-version }}-${{ matrix.numpy-version }}-${{ matrix.dependency-set }} run
+
+  test-upstream-and-min-deps:
+    name: py=${{ matrix.python-version }}-${{ matrix.dependency-set }}
+
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: ['3.11', "3.13"]
+        dependency-set: ["upstream", "min_deps"]
+        exclude:
+          - python-version: "3.13"
+            dependency-set: min_deps
+          - python-version: "3.11"
+            dependency-set: upstream
+    steps:
+    - uses: actions/checkout@v4
+    - name: Set up Python
+      uses: actions/setup-python@v5
+      with:
+        python-version: ${{ matrix.python-version }}
+        cache: 'pip'
+    - name: Install Hatch
+      run: |
+        python -m pip install --upgrade pip 
+        pip install hatch
+    - name: Set Up Hatch Env
+      run: |
+        hatch env create ${{ matrix.dependency-set }}
+        hatch env run -e ${{ matrix.dependency-set }} list-env
+    - name: Run Tests
+      run: |
+        hatch env run --env ${{ matrix.dependency-set }} run

.pre-commit-config.yaml

@@ -7,7 +7,7 @@ default_language_version:
   python: python3
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.6.9
+    rev: v0.7.0
     hooks:
     - id: ruff
       args: ["--fix", "--show-fixes"]
@@ -22,7 +22,7 @@ repos:
     hooks:
     - id: check-yaml
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.11.2
+    rev: v1.12.1
     hooks:
       - id: mypy
         files: src|tests

docs/guide/storage.rst

@@ -72,7 +72,7 @@ that implements the `AbstractFileSystem` API,
 .. code-block:: python
 
    >>> import zarr
-   >>> store = zarr.storage.RemoteStore("gs://foo/bar", mode="r")
+   >>> store = zarr.storage.RemoteStore.from_url("gs://foo/bar", mode="r")
    >>> zarr.open(store=store)
    <Array <RemoteStore(GCSFileSystem, foo/bar)> shape=(10, 20) dtype=float32>
 

pyproject.toml

@@ -28,12 +28,13 @@ requires-python = ">=3.11"
 dependencies = [
     'asciitree',
     'numpy>=1.25',
-    'numcodecs>=0.10.2',
-    'fsspec>2024',
-    'crc32c',
-    'typing_extensions',
-    'donfig',
+    'numcodecs>=0.13',
+    'fsspec>=2022.10.0',
+    'crc32c>=2.3',
+    'typing_extensions>=4.6',
+    'donfig>=0.8',
 ]
+
 dynamic = [
   "version",
 ]
@@ -98,7 +99,7 @@ extra = [
 ]
 optional = [
     'lmdb',
-    'universal-pathlib',
+    'universal-pathlib>=0.0.22',
 ]
 
 [project.urls]
@@ -183,6 +184,65 @@ features = ['docs']
 build = "cd docs && make html"
 serve = "sphinx-autobuild docs docs/_build --host 0.0.0.0"
 
+[tool.hatch.envs.upstream]
+python = "3.13"
+dependencies = [
+    'numpy',  # from scientific-python-nightly-wheels
+    'numcodecs @ git+https://github.com/zarr-developers/numcodecs',
+    'fsspec @ git+https://github.com/fsspec/filesystem_spec',
+    's3fs @ git+https://github.com/fsspec/s3fs',
+    'universal_pathlib @ git+https://github.com/fsspec/universal_pathlib',
+    'crc32c @ git+https://github.com/ICRAR/crc32c',
+    'typing_extensions @ git+https://github.com/python/typing_extensions',
+    'donfig @ git+https://github.com/pytroll/donfig',
+    # test deps
+    'hypothesis',
+    'pytest',
+    'pytest-cov',
+    'pytest-asyncio',
+    'moto[s3]',
+]
+
+[tool.hatch.envs.upstream.env-vars]
+PIP_INDEX_URL = "https://pypi.anaconda.org/scientific-python-nightly-wheels/simple/"
+PIP_EXTRA_INDEX_URL = "https://pypi.org/simple/"
+PIP_PRE = "1"
+
+[tool.hatch.envs.upstream.scripts]
+run = "pytest --verbose"
+run-mypy = "mypy src"
+run-hypothesis = "pytest --hypothesis-profile ci tests/test_properties.py tests/test_store/test_stateful*"
+list-env = "pip list"
+
+[tool.hatch.envs.min_deps]
+description = """Test environment for minimum supported dependencies
+
+See Spec 0000 for details and drop schedule: https://scientific-python.org/specs/spec-0000/
+"""
+python = "3.11"
+dependencies = [
+    'numpy==1.25.*',
+    'numcodecs==0.13.*',  # 0.13 needed for? (should be 0.11)
+    'fsspec==2022.10.0',
+    's3fs==2022.10.0',
+    'universal_pathlib==0.0.22',
+    'crc32c==2.3.*',
+    'typing_extensions==4.6.*',  # 4.5 needed for @deprecated, 4.6 for Buffer
+    'donfig==0.8.*',
+    # test deps
+    'hypothesis',
+    'pytest',
+    'pytest-cov',
+    'pytest-asyncio',
+    'moto[s3]',
+]
+
+[tool.hatch.envs.min_deps.scripts]
+run = "pytest --verbose"
+run-hypothesis = "pytest --hypothesis-profile ci tests/test_properties.py tests/test_store/test_stateful*"
+list-env = "pip list"
+
+
 [tool.ruff]
 line-length = 100
 force-exclude = true

src/zarr/abc/store.py

@@ -168,7 +168,7 @@ def with_mode(self, mode: AccessModeLiteral) -> Self:
 
         Returns
         -------
-        store:
+        store
             A new store of the same type with the new mode.
 
         Examples

src/zarr/codecs/zstd.py

@@ -3,10 +3,11 @@
 import asyncio
 from dataclasses import dataclass
 from functools import cached_property
-from importlib.metadata import version
 from typing import TYPE_CHECKING
 
+import numcodecs
 from numcodecs.zstd import Zstd
+from packaging.version import Version
 
 from zarr.abc.codec import BytesBytesCodec
 from zarr.core.buffer.cpu import as_numpy_array_wrapper
@@ -43,8 +44,8 @@ class ZstdCodec(BytesBytesCodec):
 
     def __init__(self, *, level: int = 0, checksum: bool = False) -> None:
         # numcodecs 0.13.0 introduces the checksum attribute for the zstd codec
-        _numcodecs_version = tuple(map(int, version("numcodecs").split(".")))
-        if _numcodecs_version < (0, 13, 0):  # pragma: no cover
+        _numcodecs_version = Version(numcodecs.__version__)
+        if _numcodecs_version < Version("0.13.0"):
             raise RuntimeError(
                 "numcodecs version >= 0.13.0 is required to use the zstd codec. "
                 f"Version {_numcodecs_version} is currently installed."

src/zarr/storage/remote.py

@@ -1,9 +1,8 @@
 from __future__ import annotations
 
+import warnings
 from typing import TYPE_CHECKING, Any, Self
 
-import fsspec
-
 from zarr.abc.store import ByteRangeRequest, Store
 from zarr.storage.common import _dereference_path
 
@@ -34,7 +33,8 @@ class RemoteStore(Store):
     mode : AccessModeLiteral
         The access mode to use.
     path : str
-        The root path of the store.
+        The root path of the store. This should be a relative path and must not include the
+        filesystem scheme.
     allowed_exceptions : tuple[type[Exception], ...]
         When fetching data, these cases will be deemed to correspond to missing keys.
 
@@ -46,6 +46,23 @@ class RemoteStore(Store):
     supports_deletes
     supports_partial_writes
     supports_listing
+
+    Raises
+    ------
+    TypeError
+        If the Filesystem does not support async operations.
+    ValueError
+        If the path argument includes a scheme.
+
+    Warns
+    -----
+    UserWarning
+        If the file system (fs) was not created with `asynchronous=True`.
+
+    See Also
+    --------
+    RemoteStore.from_upath
+    RemoteStore.from_url
     """
 
     # based on FSSpec
@@ -71,6 +88,15 @@ def __init__(
 
         if not self.fs.async_impl:
             raise TypeError("Filesystem needs to support async operations.")
+        if not self.fs.asynchronous:
+            warnings.warn(
+                f"fs ({fs}) was not created with `asynchronous=True`, this may lead to surprising behavior",
+                stacklevel=2,
+            )
+        if "://" in path and not path.startswith("http"):
+            # `not path.startswith("http")` is a special case for the http filesystem (¯\_(ツ)_/¯)
+            scheme, _ = path.split("://", maxsplit=1)
+            raise ValueError(f"path argument to RemoteStore must not include scheme ({scheme}://)")
 
     @classmethod
     def from_upath(
@@ -130,7 +156,23 @@ def from_url(
         -------
         RemoteStore
         """
-        fs, path = fsspec.url_to_fs(url, **storage_options)
+        try:
+            from fsspec import url_to_fs
+        except ImportError:
+            # before fsspec==2024.3.1
+            from fsspec.core import url_to_fs
+
+        opts = storage_options or {}
+        opts = {"asynchronous": True, **opts}
+
+        fs, path = url_to_fs(url, **opts)
+
+        # fsspec is not consistent about removing the scheme from the path, so check and strip it here
+        # https://github.com/fsspec/filesystem_spec/issues/1722
+        if "://" in path and not path.startswith("http"):
+            # `not path.startswith("http")` is a special case for the http filesystem (¯\_(ツ)_/¯)
+            path = fs._strip_protocol(path)
+
         return cls(fs=fs, path=path, mode=mode, allowed_exceptions=allowed_exceptions)
 
     async def clear(self) -> None:

tests/test_store/test_core.py

@@ -71,10 +71,7 @@ async def test_make_store_path_invalid() -> None:
 
 
 async def test_make_store_path_fsspec(monkeypatch) -> None:
-    import fsspec.implementations.memory
-
-    monkeypatch.setattr(fsspec.implementations.memory.MemoryFileSystem, "async_impl", True)
-    store_path = await make_store_path("memory://")
+    store_path = await make_store_path("http://foo.com/bar")
     assert isinstance(store_path.store, RemoteStore)
 
 

tests/test_store/test_remote.py

@@ -86,10 +86,12 @@ def s3(s3_base: None) -> Generator[s3fs.S3FileSystem, None, None]:
 
 async def test_basic() -> None:
     store = RemoteStore.from_url(
-        f"s3://{test_bucket_name}",
+        f"s3://{test_bucket_name}/foo/spam/",
         mode="w",
         storage_options={"endpoint_url": endpoint_url, "anon": False},
     )
+    assert store.fs.asynchronous
+    assert store.path == f"{test_bucket_name}/foo/spam"
     assert await _collect_aiterator(store.list()) == ()
     assert not await store.exists("foo")
     data = b"hello"
@@ -109,7 +111,7 @@ class TestRemoteStoreS3(StoreTests[RemoteStore, cpu.Buffer]):
     @pytest.fixture
     def store_kwargs(self, request) -> dict[str, str | bool]:
         fs, path = fsspec.url_to_fs(
-            f"s3://{test_bucket_name}", endpoint_url=endpoint_url, anon=False
+            f"s3://{test_bucket_name}", endpoint_url=endpoint_url, anon=False, asynchronous=True
         )
         return {"fs": fs, "path": path, "mode": "r+"}
 
@@ -143,9 +145,7 @@ def test_store_supports_partial_writes(self, store: RemoteStore) -> None:
     def test_store_supports_listing(self, store: RemoteStore) -> None:
         assert store.supports_listing
 
-    async def test_remote_store_from_uri(
-        self, store: RemoteStore, store_kwargs: dict[str, str | bool]
-    ):
+    async def test_remote_store_from_uri(self, store: RemoteStore):
         storage_options = {
             "endpoint_url": endpoint_url,
             "anon": False,
@@ -183,9 +183,32 @@ async def test_remote_store_from_uri(
         assert dict(group.attrs) == {"key": "value-3"}
 
     def test_from_upath(self) -> None:
-        path = UPath(f"s3://{test_bucket_name}", endpoint_url=endpoint_url, anon=False)
+        path = UPath(
+            f"s3://{test_bucket_name}/foo/bar/",
+            endpoint_url=endpoint_url,
+            anon=False,
+            asynchronous=True,
+        )
         result = RemoteStore.from_upath(path)
         assert result.fs.endpoint_url == endpoint_url
+        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:
+        # regression test for https://github.com/zarr-developers/zarr-python/issues/2342
+        store_kwargs["path"] = "s3://" + store_kwargs["path"]
+        with pytest.raises(
+            ValueError, match="path argument to RemoteStore must not include scheme .*"
+        ):
+            self.store_cls(**store_kwargs)
+
+    def test_init_warns_if_fs_asynchronous_is_false(self) -> None:
+        fs, path = fsspec.url_to_fs(
+            f"s3://{test_bucket_name}", endpoint_url=endpoint_url, anon=False, asynchronous=False
+        )
+        store_kwargs = {"fs": fs, "path": path, "mode": "r+"}
+        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:
         # regression test for https://github.com/zarr-developers/zarr-python/pull/2343