Merge branch 'main' into unsigned-buffer

d-v-b · web-flow · commit 53d4533b9c51 · 2025-01-25T18:30:08.000+01:00
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -4,6 +4,6 @@ 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/user-guide/*.rst`
-* [ ] Changes documented in `docs/release-notes.rst`
+* [ ] Changes documented as a new file in `changes/`
 * [ ] GitHub Actions have all passed
 * [ ] Test coverage is 100% (Codecov passes)
diff --git a/.github/labeler.yml b/.github/labeler.yml
@@ -1,2 +1,4 @@
 needs release notes:
-- all: ['!docs/release-notes.rst']
+  - all:
+    - changed-files:
+      - any-glob-to-any-file: '!changes/*.rst'
diff --git a/.github/workflows/needs_release_notes.yml b/.github/workflows/needs_release_notes.yml
@@ -4,8 +4,11 @@ on:
   - pull_request_target
 
 jobs:
-  triage:
+  labeler:
     if: ${{ github.event.pull_request.user.login != 'dependabot[bot]' }} && ${{ github.event.pull_request.user.login != 'pre-commit-ci[bot]' }}
+    permissions:
+      contents: read
+      pull-requests: write
     runs-on: ubuntu-latest
     steps:
     - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9  # v5.0.0
diff --git a/.gitignore b/.gitignore
@@ -83,6 +83,7 @@ src/zarr/_version.py
 data/*
 src/fixture/
 fixture/
+junit.xml
 
 .DS_Store
 tests/.hypothesis
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -49,3 +49,7 @@ repos:
     rev: v1.8.0
     hooks:
       - id: numpydoc-validation
+  - repo: https://github.com/twisted/towncrier
+    rev: 23.11.0
+    hooks:
+      - id: towncrier-check
diff --git a/.readthedocs.yaml b/.readthedocs.yaml
@@ -4,6 +4,13 @@ build:
   os: ubuntu-22.04
   tools:
     python: "3.12"
+  jobs:
+    pre_build:
+      - |
+        if [ "$READTHEDOCS_VERSION_TYPE" != "tag" ];
+        then
+          towncrier build --version Unreleased --yes;
+        fi
 
 sphinx:
   configuration: docs/conf.py
diff --git a/changes/.gitignore b/changes/.gitignore
@@ -0,0 +1 @@
+!.gitignore
diff --git a/changes/2533.bigfix.rst b/changes/2533.bigfix.rst
@@ -0,0 +1 @@
+Wrap sync fsspec filesystems with AsyncFileSystemWrapper in xarray.to_zarr
diff --git a/changes/2681.bugfix.rst b/changes/2681.bugfix.rst
@@ -0,0 +1 @@
+Added backwards compatibility for Zarr format 2 structured arrays.
diff --git a/changes/2693.bugfix.rst b/changes/2693.bugfix.rst
@@ -0,0 +1,13 @@
+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 '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
+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
diff --git a/changes/2736.doc.rst b/changes/2736.doc.rst
@@ -0,0 +1,2 @@
+Changed the machinery for creating changelog entries.
+Now individual entries should be added as files to the `changes` directory in the `zarr-python` repository, instead of directly to the changelog file.
diff --git a/changes/2762.bugfix.rst b/changes/2762.bugfix.rst
@@ -0,0 +1,2 @@
+Fixed ZipStore to make sure the correct attributes are saved when instances are pickled.
+This fixes a previous bug that prevent using ZipStore with a ProcessPoolExecutor.
diff --git a/changes/README.md b/changes/README.md
@@ -0,0 +1,14 @@
+Writing a changelog entry
+-------------------------
+
+Please put a new file in this directory named `xxxx.<type>.rst`, where
+
+- `xxxx` is the pull request number associated with this entry
+- `<type>` is one of:
+  - feature
+  - bugfix
+  - doc
+  - removal
+  - misc
+
+Inside the file, please write a short description of what you have changed, and how it impacts users of `zarr-python`.
diff --git a/docs/developers/contributing.rst b/docs/developers/contributing.rst
@@ -216,8 +216,8 @@ The documentation consists both of prose and API documentation. All user-facing
 and functions are included in the API documentation, under the ``docs/api`` folder
 using the `autodoc <https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html>`_
 extension to sphinx. Any new features or important usage information should be included in the
-user-guide (``docs/user-guide``). Any changes should also be included in the release
-notes (``docs/release-notes.rst``).
+user-guide (``docs/user-guide``). Any changes should also be included as a new file in the
+:file:`changes` directory.
 
 The documentation can be built locally by running::
 
@@ -335,11 +335,9 @@ Release procedure
 
 Pre-release
 """""""""""
-1. Make sure that all pull requests which will be
-   included in the release have been properly documented in
-   :file:`docs/release-notes.rst`.
-2. Rename the "Unreleased" section heading in :file:`docs/release-notes.rst`
-   to the version you are about to release.
+1. Make sure that all pull requests which will be included in the release
+   have been properly documented as changelog files in :file:`changes`.
+2. Run ``towncrier build --version x.y.z`` to create the changelog.
 
 Releasing
 """""""""
@@ -352,7 +350,7 @@ appropriate suffix (e.g. `v0.0.0a1` or `v0.0.0rc2`).
 
 Set the description of the release to::
 
-    See release notes https://zarr.readthedocs.io/en/stable/release.html#release-0-0-0
+    See release notes https://zarr.readthedocs.io/en/stable/release-notes.html#release-0-0-0
 
 replacing the correct version numbers. For pre-release versions,
 the URL should omit the pre-release suffix, e.g. "a1" or "rc1".
diff --git a/docs/release-notes.rst b/docs/release-notes.rst
@@ -1,22 +1,8 @@
 Release notes
 =============
 
-Unreleased
-----------
+.. towncrier release notes start
 
-Bug fixes
-~~~~~~~~~
-
-* Backwards compatibility for Zarr format 2 structured arrays (:issue:`2134`)
-
-Features
-~~~~~~~~
-
-Documentation
-~~~~~~~~~~~~~
-
-Other
-~~~~~
 
 3.0.1 (Jan. 17, 2025)
 ---------------------
diff --git a/pyproject.toml b/pyproject.toml
@@ -85,6 +85,7 @@ test = [
 ]
 optional = ["rich", "universal-pathlib"]
 docs = [
+    # Doc building
     'sphinx==8.1.3',
     'sphinx-autobuild>=2021.3.14',
     'sphinx-autoapi==3.4.0',
@@ -94,6 +95,9 @@ docs = [
     'sphinx-reredirects',
     'pydata-sphinx-theme',
     'numpydoc',
+    # Changelog generation
+    'towncrier',
+    # Optional dependencies to run examples
     'numcodecs[msgpack]',
     'rich',
     's3fs',
@@ -415,3 +419,9 @@ checks = [
     "PR05",
     "PR06",
 ]
+
+[tool.towncrier]
+directory = 'changes'
+filename = "docs/release-notes.rst"
+underlines = ["-", "~", "^"]
+issue_format = ":issue:`{issue}`"
diff --git 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
 
diff --git a/src/zarr/storage/_fsspec.py b/src/zarr/storage/_fsspec.py
@@ -10,14 +10,15 @@
     Store,
     SuffixByteRequest,
 )
+from zarr.core.buffer import Buffer
 from zarr.storage._common import _dereference_path
 
 if TYPE_CHECKING:
     from collections.abc import AsyncIterator, Iterable
 
     from fsspec.asyn import AsyncFileSystem
 
-    from zarr.core.buffer import Buffer, BufferPrototype
+    from zarr.core.buffer import BufferPrototype
     from zarr.core.common import BytesLike
 
 
@@ -172,6 +173,17 @@ def from_url(
         opts = {"asynchronous": True, **opts}
 
         fs, path = url_to_fs(url, **opts)
+        if not fs.async_impl:
+            try:
+                from fsspec.implementations.asyn_wrapper import AsyncFileSystemWrapper
+
+                fs = AsyncFileSystemWrapper(fs)
+            except ImportError as e:
+                raise ImportError(
+                    f"The filesystem for URL '{url}' is synchronous, and the required "
+                    "AsyncFileSystemWrapper is not available. Upgrade fsspec to version "
+                    "2024.12.0 or later to enable this functionality."
+                ) from e
 
         # 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
@@ -253,6 +265,10 @@ async def set(
         if not self._is_open:
             await self._open()
         self._check_writable()
+        if not isinstance(value, Buffer):
+            raise TypeError(
+                f"FsspecStore.set(): `value` must be 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/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
 
@@ -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/_logging.py b/src/zarr/storage/_logging.py
@@ -2,10 +2,11 @@
 
 import inspect
 import logging
+import sys
 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,8 +19,10 @@
 
     counter: defaultdict[str, int]
 
+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.
 
@@ -42,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:
@@ -67,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")
@@ -94,6 +97,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():
@@ -126,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():
@@ -151,11 +161,11 @@ 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):
-            return self._store == other
+            return type(self) is type(other) and self._store.__eq__(other._store)  # type: ignore[attr-defined]
 
     async def get(
         self,
diff --git 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]
@@ -231,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)
diff --git a/src/zarr/storage/_wrapper.py b/src/zarr/storage/_wrapper.py
diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py
diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py
diff --git a/tests/test_store/test_core.py b/tests/test_store/test_core.py
diff --git a/tests/test_store/test_fsspec.py b/tests/test_store/test_fsspec.py
diff --git a/tests/test_store/test_local.py b/tests/test_store/test_local.py
diff --git a/tests/test_store/test_logging.py b/tests/test_store/test_logging.py
diff --git a/tests/test_store/test_wrapper.py b/tests/test_store/test_wrapper.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Wrap sync fsspec filesystems with AsyncFileSystemWrapper in xarray.to_zarr`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Added backwards compatibility for Zarr format 2 structured arrays.`