fix: informative errors for None in chunk specifications

docs/release-notes.md

@@ -37,6 +37,13 @@
   the array's shape), and `None` as a per-dimension chunk size. These all now
   raise informative errors. Also fix chunk handling for 0-length array dimensions,
   and add explicit rejection of 0-length chunks. ([#3899](https://github.com/zarr-developers/zarr-python/issues/3899))
+- Further hardened chunk normalization: a per-dimension `None` chunk size now
+  raises an informative `ValueError` directing users to the `-1` sentinel
+  (previously an uninformative `TypeError`), per-dimension boolean chunk sizes
+  are rejected instead of `True` silently producing size-1 chunks, and strings
+  and other non-iterable chunk inputs raise informative `TypeError`s instead of
+  bare crashes. Generator inputs to chunk normalization are now materialized
+  and accepted. ([#4177](https://github.com/zarr-developers/zarr-python/issues/4177))
 - Handle missing consolidated metadata in leaf Group nodes. ([#3954](https://github.com/zarr-developers/zarr-python/issues/3954))
 - Corrected the JSON type definitions for the `numpy.datetime64` and
   `numpy.timedelta64` data types in Zarr V3 metadata: the `configuration` object

src/zarr/core/array.py

@@ -4461,7 +4461,17 @@ async def init_array(
                 "chunks=(inner_size, ...), shards=[[shard_sizes], ...]"
             )
 
-    # Normalize the user's chunks into canonical ChunksTuple form
+    # Normalize the user's chunks into canonical ChunksTuple form.
+    # Auto-chunking is an API-level concept, so the guidance toward it is
+    # raised here rather than in the mechanical normalizer. Validate through
+    # an object-typed view: None/True are outside ChunksLike but reachable
+    # from untyped callers.
+    chunks_input: object = chunks
+    if chunks_input is None or chunks_input is True:
+        raise ValueError(
+            f'{chunks!r} is not a valid chunk input. Use chunks="auto" or omit the chunks '
+            "argument for automatic chunking, or pass an int / iterable of ints."
+        )
 
     if chunks == "auto":
         max_bytes = None if shards is None else SHARDED_INNER_CHUNK_MAX_BYTES

src/zarr/core/chunk_grids.py

@@ -6,6 +6,7 @@
 import numbers
 import operator
 import warnings
+from collections.abc import Iterable
 from dataclasses import dataclass, field
 from functools import reduce
 from typing import (
@@ -31,7 +32,7 @@
 from zarr.errors import ZarrUserWarning
 
 if TYPE_CHECKING:
-    from collections.abc import Iterable, Iterator, Sequence
+    from collections.abc import Iterator, Sequence
 
     from zarr.core.array import ShardsLike
     from zarr.core.metadata import ArrayMetadata
@@ -717,18 +718,33 @@ def _guess_regular_chunks(
     return tuple(int(x) for x in chunks)
 
 
-def normalize_chunks_1d(
-    chunks: int | Iterable[object], span: int
-) -> np.ndarray[tuple[int], np.dtype[np.int64]]:
+def normalize_chunks_1d(chunks: object, span: int) -> np.ndarray[tuple[int], np.dtype[np.int64]]:
     """
     Normalize a one-dimensional chunk specification into a 1D int64 array of
     chunk sizes that cover the span.
 
-    `-1` means "one chunk covering the entire span."
+    Accepts `object` and narrows internally: `None` and `bool` are rejected
+    with informative errors (`-1` is the sentinel for "one chunk covering the
+    entire span"), strings and other non-iterables are rejected, and any other
+    iterable (including a generator) is materialized as an explicit list of
+    chunk sizes.
     For an integer chunk size, all chunks are uniform — the last chunk may
     overhang the span. The actual data extent of each chunk is determined
     by the chunk grid at runtime, not by this function.
     """
+    if chunks is None:
+        raise ValueError(
+            "None is not a valid chunk size for a dimension. "
+            "Use -1 for a single chunk covering the full extent of an axis."
+        )
+    # bool is a subclass of int, so without this guard True would silently
+    # pass through the integer branch below as chunk size 1.
+    if chunks is True or chunks is False:
+        raise ValueError(
+            f"{chunks} is not a valid chunk size for a dimension. "
+            "Chunk sizes must be positive integers, or -1 for a single chunk "
+            "covering the full extent of an axis."
+        )
     if chunks == -1:
         return np.array([span], dtype=np.int64)
     if isinstance(chunks, int):
@@ -738,39 +754,45 @@ def normalize_chunks_1d(
             return np.array([chunks], dtype=np.int64)
         n = ceildiv(span, chunks)
         return np.full(n, chunks, dtype=np.int64)
-    else:
-        chunk_list = list(chunks)
-        if not chunk_list:
-            raise ValueError("Chunk specification must not be empty")
-        non_int = [
-            (idx, c) for idx, c in enumerate(chunk_list) if not isinstance(c, numbers.Integral)
-        ]
-        if non_int:
-            non_int_idxs, non_int_vals = [*zip(*non_int, strict=False)]
-            raise TypeError(
-                f"Each chunk size must be an integer; got non-integer element(s) {non_int_vals!r} "
-                f"at indices {non_int_idxs!r}. Chunk sizes must be declared as a flat sequence of "
-                f"positive integers (e.g. [3, 3, 1])."
-            )
-        ints: list[int] = [int(c) for c in chunk_list]  # type: ignore[call-overload]
-        if any(c <= 0 for c in ints):
-            raise ValueError(f"All chunk sizes must be positive, got {ints}")
-        if sum(ints) != span:
-            raise ValueError(f"Chunk sizes {ints} do not sum to span {span}")
-        return np.asarray(ints, dtype=np.int64)
+    # str/bytes are iterable but never a valid chunk specification
+    if isinstance(chunks, (str, bytes)) or not isinstance(chunks, Iterable):
+        raise TypeError(
+            f"{chunks!r} is not a valid chunk size for a dimension. "
+            "Expected an int or an iterable of ints."
+        )
+    chunk_list = list(chunks)
+    if not chunk_list:
+        raise ValueError("Chunk specification must not be empty")
+    non_int = [(idx, c) for idx, c in enumerate(chunk_list) if not isinstance(c, numbers.Integral)]
+    if non_int:
+        non_int_idxs, non_int_vals = [*zip(*non_int, strict=False)]
+        raise TypeError(
+            f"Each chunk size must be an integer; got non-integer element(s) {non_int_vals!r} "
+            f"at indices {non_int_idxs!r}. Chunk sizes must be declared as a flat sequence of "
+            f"positive integers (e.g. [3, 3, 1])."
+        )
+    ints: list[int] = [int(c) for c in chunk_list]
+    if any(c <= 0 for c in ints):
+        raise ValueError(f"All chunk sizes must be positive, got {ints}")
+    if sum(ints) != span:
+        raise ValueError(f"Chunk sizes {ints} do not sum to span {span}")
+    return np.asarray(ints, dtype=np.int64)
 
 
 def normalize_chunks_nd(
-    chunks: Any,
+    chunks: object,
     shape: tuple[int, ...],
 ) -> ChunksTuple:
     """
     Normalize a chunk specification into a `ChunksTuple`.
 
     This is a mechanical transformation — no heuristics, no guessing.
-    Handles `False` ("all data in one chunk"), scalar ints, `-1` sentinels (one chunk
-    per dimension covering the full span), and explicit per-dimension lists
-    of chunk sizes (regular or rectilinear).
+    Accepts `object` and narrows internally: `False` ("all data in one chunk"),
+    scalar ints, `-1` sentinels (one chunk per dimension covering the full
+    span), and per-dimension iterables of chunk sizes (regular or
+    rectilinear). Any non-string iterable — including a generator — is
+    materialized before use; strings and non-iterables are rejected with
+    informative errors.
 
     For auto-chunking, use `guess_chunks` which returns a
     `ChunksTuple` directly. `chunks=None` and `chunks=True` are rejected
@@ -779,7 +801,8 @@ def normalize_chunks_nd(
     """
     if chunks is None or chunks is True:
         raise ValueError(
-            f'{chunks!r} is not a valid chunk input. Use chunks=None or chunks="auto" from the top-level API for auto-chunking, or pass an int / tuple of ints.'
+            f"{chunks!r} is not a valid chunk input. "
+            "Expected an int, an iterable of ints, or False."
         )
 
     # handle no chunking
@@ -788,16 +811,24 @@ def normalize_chunks_nd(
 
     # handle 1D convenience form. bool is excluded above so this only catches actual ints.
     if isinstance(chunks, numbers.Integral):
-        chunks = tuple(int(chunks) for _ in shape)
+        chunks_tuple: tuple[Any, ...] = tuple(int(chunks) for _ in shape)
+    elif isinstance(chunks, Iterable) and not isinstance(chunks, (str, bytes)):
+        # materialize before use so generators are supported and len() is safe;
+        # str/bytes are iterable but never a valid chunk specification
+        chunks_tuple = tuple(chunks)
+    else:
+        raise TypeError(
+            f"{chunks!r} is not a valid chunk input. Expected an int or an iterable of ints."
+        )
 
     # handle bad dimensionality
-    if len(chunks) != len(shape):
+    if len(chunks_tuple) != len(shape):
         raise ValueError(
-            f"chunks has {len(chunks)} dimensions but shape has {len(shape)} dimensions"
+            f"chunks has {len(chunks_tuple)} dimensions but shape has {len(shape)} dimensions"
         )
 
     return ChunksTuple(
-        tuple(normalize_chunks_1d(c, span=s) for c, s in zip(chunks, shape, strict=True))
+        tuple(normalize_chunks_1d(c, span=s) for c, s in zip(chunks_tuple, shape, strict=True))
     )
 
 

src/zarr/storage/_fsspec.py

@@ -3,7 +3,6 @@
 import json
 import warnings
 from contextlib import suppress
-from logging import getLogger
 from typing import TYPE_CHECKING, Any
 
 from packaging.version import parse as parse_version
@@ -19,8 +18,6 @@
 from zarr.errors import ZarrUserWarning
 from zarr.storage._utils import _dereference_path
 
-logger = getLogger(__name__)
-
 if TYPE_CHECKING:
     from collections.abc import AsyncIterator, Iterable
 
@@ -38,26 +35,6 @@
 )
 
 
-async def _close_fs(fs: AsyncFileSystem) -> None:
-    """
-    Best-effort async close of an fsspec async filesystem owned by FsspecStore.
-
-    For filesystems that expose `set_session()` (e.g. s3fs) the underlying
-    aiohttp `ClientSession` is closed explicitly, which prevents
-    "Unclosed client session" `ResourceWarning`s from aiohttp.  For all
-    other filesystem types the call is a no-op (not every implementation
-    manages an HTTP session directly).
-
-    Note that `set_session()` lazily creates a session if none exists yet, so
-    closing a store that never performed any I/O may instantiate a session
-    purely to close it.  This is accepted best-effort behavior; fsspec does not
-    expose a stable, cross-implementation way to test for an existing session.
-    """
-    if hasattr(fs, "set_session"):
-        session = await fs.set_session()
-        await session.close()
-
-
 def _make_async(fs: AbstractFileSystem) -> AsyncFileSystem:
     """Convert a sync FSSpec filesystem to an async FFSpec filesystem
 
@@ -126,6 +103,15 @@ class FsspecStore(Store):
     ZarrUserWarning
         If the file system (fs) was not created with `asynchronous=True`.
 
+    Notes
+    -----
+    Closing the store does not close the underlying filesystem or its network
+    session. fsspec caches and shares filesystem instances across callers, so
+    the store cannot know whether it is the only user, and closing a shared
+    session would break other stores. The filesystem's lifecycle belongs to
+    whoever created it; use fsspec's own tools (e.g. `clear_instance_cache`)
+    to release it.
+
     See Also
     --------
     FsspecStore.from_upath
@@ -152,9 +138,6 @@ def __init__(
         self.fs = fs
         self.path = path
         self.allowed_exceptions = allowed_exceptions
-        # True only when this store created fs itself (from_url / from_mapper with new instance).
-        # Callers who supply their own fs remain responsible for its lifecycle.
-        self._owns_fs: bool = False
 
         if not self.fs.async_impl:
             raise TypeError("Filesystem needs to support async operations.")
@@ -220,17 +203,13 @@ def from_mapper(
         -------
         FsspecStore
         """
-        original_fs = fs_map.fs
-        fs = _make_async(original_fs)
-        store = cls(
+        fs = _make_async(fs_map.fs)
+        return cls(
             fs=fs,
             path=fs_map.root,
             read_only=read_only,
             allowed_exceptions=allowed_exceptions,
         )
-        # _make_async returns a new instance when converting sync→async; own it.
-        store._owns_fs = fs is not original_fs
-        return store
 
     @classmethod
     def from_url(
@@ -272,39 +251,16 @@ def from_url(
         if not fs.async_impl:
             fs = _make_async(fs)
 
-        store = cls(fs=fs, path=path, read_only=read_only, allowed_exceptions=allowed_exceptions)
-        store._owns_fs = True
-        return store
+        return cls(fs=fs, path=path, read_only=read_only, allowed_exceptions=allowed_exceptions)
 
     def with_read_only(self, read_only: bool = False) -> FsspecStore:
         # docstring inherited
-        new_store = type(self)(
+        return type(self)(
             fs=self.fs,
             path=self.path,
             allowed_exceptions=self.allowed_exceptions,
             read_only=read_only,
         )
-        # The derived store shares the same fs. Transfer ownership so the
-        # surviving store closes it, and clear ours to avoid a double-close.
-        # Otherwise the common `from_url(...).with_read_only()` chain would
-        # drop the only owner (the unreferenced source) and leak the session.
-        new_store._owns_fs = self._owns_fs
-        self._owns_fs = False
-        return new_store
-
-    def close(self) -> None:
-        # docstring inherited
-        if self._owns_fs:
-            from zarr.core.sync import sync as zarr_sync
-
-            # Best-effort: a failure to release the session must not block close(),
-            # but log it so a genuine regression in the close path stays observable
-            # rather than silently reverting to the leaking behavior.
-            try:
-                zarr_sync(_close_fs(self.fs))
-            except Exception:
-                logger.debug("Failed to close owned filesystem %r", self.fs, exc_info=True)
-        super().close()
 
     async def clear(self) -> None:
         # docstring inherited

tests/test_api.py

@@ -79,7 +79,7 @@ def test_create(memory_store: Store) -> None:
         z = create(shape=(400.5, 100), store=store, overwrite=True)  # type: ignore[arg-type]
 
     # create array with float chunk shape
-    with pytest.raises(TypeError, match="'float' object is not iterable"):
+    with pytest.raises(TypeError, match="is not a valid chunk size for a dimension"):
         z = create(shape=(400, 100), chunks=(16, 16.5), store=store, overwrite=True)  # type: ignore[arg-type]