From 4e391851516ff3ab1385702b7f02e0709e0f5488 Mon Sep 17 00:00:00 2001 From: arcusbuilds Date: Thu, 20 Aug 2026 11:05:51 +0530 Subject: [PATCH 1/3] feat: name packages that provide a codec zarr cannot find Closes #4271. When zarr fails to resolve a codec it now says which Python packages are known to provide it, instead of raising a bare KeyError holding only the codec name: An implementation for codec 'wavpack' is not available. Register one explicitly using the codec registry (see ), or install a Python package that registers a codec implementation with numcodecs. Known packages supporting this codec: wavpack-numcodecs. Two hand-maintained tables in zarr/registry.py hold the mapping, one per Zarr format, because the two formats resolve codecs through different registries and the same name can mean different things in each: `imagecodecs_*` names are declared by `virtual-tiff` under the `zarr.codecs` entry point group and by `imagecodecs-numcodecs` under `numcodecs.codecs`, and `crc32c` is a codec zarr implements itself in format 3 while in format 2 it needs `numcodecs[crc32c]`. Each table has an exact-match and a prefix-match half, since packages that provide many codecs namespace them behind a shared prefix. Entries cover third-party packages and the codecs numcodecs gates behind its own optional dependencies -- `zfpy`, `pcodec`, `crc32c` and `msgpack2` -- which are the most common missing-codec case in practice. Backwards compatibility: `get_codec_class` now raises `zarr.errors.UnknownCodecError` instead of `KeyError`, both for a codec with no registered implementation and for a codec whose configured implementation is not registered. `get_numcodec` raises it instead of the ValueError numcodecs raises for an unregistered format 2 codec id. All are subclasses of `ValueError`. Carrying the message on a `KeyError` was not an option: `KeyError.__str__` reprs its argument, so a multi-sentence message comes back quoted and escaped. `UnknownCodecError` is now exported from `zarr.errors`, since users are being told to catch it. `get_numcodec` supports numcodecs down to the declared 0.14 floor: `numcodecs.errors` only exists from 0.15.1, so the unregistered-codec check prefers that exception type where it is importable and falls back to matching the message otherwise. Signed-off-by: arcusbuilds --- changes/4277.feature.md | 14 ++ docs/user-guide/extending.md | 7 + src/zarr/core/metadata/v3.py | 7 +- src/zarr/errors.py | 1 + src/zarr/metadata/migrate_v3.py | 3 +- src/zarr/registry.py | 162 ++++++++++++++++- tests/test_codecs/test_numcodecs.py | 14 +- tests/test_config.py | 4 +- tests/test_registry.py | 260 ++++++++++++++++++++++++++++ 9 files changed, 443 insertions(+), 29 deletions(-) create mode 100644 changes/4277.feature.md create mode 100644 tests/test_registry.py diff --git a/changes/4277.feature.md b/changes/4277.feature.md new file mode 100644 index 0000000000..03caaadce3 --- /dev/null +++ b/changes/4277.feature.md @@ -0,0 +1,14 @@ +When Zarr cannot find an implementation for a codec, the error now names Python packages known to +provide that codec, e.g. `An implementation for codec 'wavpack' is not available. Register one +explicitly using the codec registry (see ...), or install a Python package that registers a codec +implementation with numcodecs. Known packages supporting this codec: wavpack-numcodecs.` This +covers codecs that `numcodecs` itself gates behind an optional dependency — `zfpy`, `pcodec`, +`crc32c` and `msgpack2` now point at the matching `numcodecs[...]` extra — as well as third-party +packages. Codec authors can add their published package to the table in `zarr/registry.py`. + +`zarr.registry.get_codec_class` now raises `zarr.errors.UnknownCodecError` instead of `KeyError`, +both for a codec with no registered implementation and for a codec whose configured +implementation is not registered. `zarr.registry.get_numcodec` raises it instead of the +`ValueError` (`numcodecs.errors.UnknownCodecError` on numcodecs 0.15.1 and later) that numcodecs +raises for an unregistered Zarr format 2 codec id. All of these are subclasses of `ValueError`. +`zarr.errors.UnknownCodecError` is now exported from `zarr.errors`. diff --git a/docs/user-guide/extending.md b/docs/user-guide/extending.md index f852f9105e..6be98ca4e5 100644 --- a/docs/user-guide/extending.md +++ b/docs/user-guide/extending.md @@ -63,6 +63,13 @@ New codecs need to have their own unique identifier. To avoid naming collisions, strongly recommended to prefix the codec identifier with a unique name. For example, the codecs from `numcodecs` are prefixed with `numcodecs.`, e.g. `numcodecs.delta`. +If someone opens an array that uses your codec without your package installed, Zarr raises +[`zarr.errors.UnknownCodecError`][] explaining how to register an implementation. Zarr also +keeps a small table of codec names and the published packages that provide them, and names +those packages in that error. Once your package is on PyPI, please open a pull request adding +it to the codec-package table in `zarr/registry.py`, so that users get a message telling them +exactly what to install. + !!! note Note that the extension mechanism for the Zarr format 3 is still under development. Requirements for custom codecs including the choice of codec identifiers might diff --git a/src/zarr/core/metadata/v3.py b/src/zarr/core/metadata/v3.py index fc47f8fc95..a73d4aa533 100644 --- a/src/zarr/core/metadata/v3.py +++ b/src/zarr/core/metadata/v3.py @@ -36,7 +36,7 @@ from zarr.core.dtype.common import check_dtype_spec_v3 from zarr.core.json_parse import parse_field, validate_json_value from zarr.core.metadata.common import parse_attributes -from zarr.errors import MetadataValidationError, NodeTypeValidationError, UnknownCodecError +from zarr.errors import MetadataValidationError, NodeTypeValidationError from zarr.registry import get_codec_class if TYPE_CHECKING: @@ -74,10 +74,7 @@ def parse_codecs(data: object) -> tuple[Codec, ...]: else: name_parsed, _ = parse_named_configuration(c, require_configuration=False) - try: - out += (get_codec_class(name_parsed).from_dict(c),) - except KeyError as e: - raise UnknownCodecError(f"Unknown codec: {e.args[0]!r}") from e + out += (get_codec_class(name_parsed).from_dict(c),) return out diff --git a/src/zarr/errors.py b/src/zarr/errors.py index 781bebe534..3e445de2e9 100644 --- a/src/zarr/errors.py +++ b/src/zarr/errors.py @@ -12,6 +12,7 @@ "MetadataValidationError", "NegativeStepError", "NodeTypeValidationError", + "UnknownCodecError", "UnstableSpecificationWarning", "VindexInvalidSelectionError", "ZarrDeprecationWarning", diff --git a/src/zarr/metadata/migrate_v3.py b/src/zarr/metadata/migrate_v3.py index 370af75a6d..ad177e19fb 100644 --- a/src/zarr/metadata/migrate_v3.py +++ b/src/zarr/metadata/migrate_v3.py @@ -29,6 +29,7 @@ from zarr.core.metadata.v2 import ArrayV2Metadata from zarr.core.metadata.v3 import ArrayV3Metadata, RegularChunkGridMetadata from zarr.core.sync import sync +from zarr.errors import UnknownCodecError from zarr.registry import get_codec_class from zarr.storage import StorePath from zarr.types import AnyArray @@ -273,7 +274,7 @@ def _find_numcodecs_zarr3(numcodecs_codec: numcodecs.abc.Codec) -> Codec: try: codec_v3 = get_codec_class(numcodec_name) - except KeyError as exc: + except UnknownCodecError as exc: raise ValueError( f"Couldn't find corresponding zarr.codecs.numcodecs codec for {numcodecs_codec.codec_id}" ) from exc diff --git a/src/zarr/registry.py b/src/zarr/registry.py index c2c0eb2921..d4ad47de7b 100644 --- a/src/zarr/registry.py +++ b/src/zarr/registry.py @@ -7,7 +7,7 @@ from zarr.core.config import BadConfigError, config from zarr.core.dtype import data_type_registry -from zarr.errors import ZarrUserWarning +from zarr.errors import UnknownCodecError, ZarrUserWarning if TYPE_CHECKING: from importlib.metadata import EntryPoint @@ -23,7 +23,7 @@ from zarr.abc.numcodec import Numcodec from zarr.core.buffer import Buffer, NDBuffer from zarr.core.chunk_key_encodings import ChunkKeyEncoding - from zarr.core.common import JSON + from zarr.core.common import JSON, ZarrFormat __all__ = [ "Registry", @@ -39,6 +39,133 @@ "register_pipeline", ] +_ZARR_CODEC_DOCS_URL = "https://zarr.readthedocs.io/en/stable/user-guide/extending/#custom-codecs" +_NUMCODECS_CODEC_DOCS_URL = ( + "https://numcodecs.readthedocs.io/en/stable/registry.html#numcodecs.registry.register_codec" +) + +# Codecs zarr-python does not implement, mapped to the names of Python packages that do. +# These tables exist purely to make the "no implementation for this codec" error actionable; +# nothing here affects which codecs zarr can actually read or write. Values are what you would +# pass to `pip install`. Only add an entry you have verified against the package's declared +# entry points, and only for a package that is actually published. +# +# The two Zarr formats resolve codecs through different registries, so they get different +# tables: a name can mean one thing as a Zarr format 3 codec name and another as a Zarr +# format 2 codec id. `imagecodecs_*` is exactly that -- `virtual-tiff` declares those names +# under `zarr.codecs`, while `imagecodecs-numcodecs` declares them under `numcodecs.codecs`. + +# Zarr format 3 codec names (entry point group "zarr.codecs"). +_CODEC_PACKAGES: dict[str, tuple[str, ...]] = { + "gribberish": ("gribberish",), + "n5_default": ("zarr-n5",), +} + +# As `_CODEC_PACKAGES`, but each key is matched against the start of the codec name. Packages +# that provide many codecs namespace them behind a shared prefix, so one entry covers them all. +_CODEC_PACKAGE_PREFIXES: dict[str, tuple[str, ...]] = { + "any-numcodecs.": ("zarr-any-numcodecs",), + "imagecodecs_": ("virtual-tiff",), + "omfiles.": ("omfiles",), + "virtual_tiff.": ("virtual-tiff",), +} + +# Zarr format 2 codec ids (entry point group "numcodecs.codecs"). `numcodecs` itself gates +# several of its own codecs behind optional dependencies, so the package to install for those +# is an extra of numcodecs rather than a third-party distribution. +_NUMCODEC_PACKAGES: dict[str, tuple[str, ...]] = { + "FITSAscii": ("kerchunk",), + "FITSVarBintable": ("kerchunk",), + "crc32c": ("numcodecs[crc32c]",), + "fill_hdf_strings": ("kerchunk",), + "grib": ("kerchunk",), + "msgpack2": ("numcodecs[msgpack]",), + "pcodec": ("numcodecs[pcodec]",), + "rawgrib": ("gribscan",), + "record_member": ("kerchunk",), + "vc-delta3d": ("vc-delta3d",), + "wavpack": ("wavpack-numcodecs",), + "zfpy": ("numcodecs[zfpy]",), +} + +# As `_NUMCODEC_PACKAGES`, but matched against the start of the codec id. +_NUMCODEC_PACKAGE_PREFIXES: dict[str, tuple[str, ...]] = { + "gribscan.": ("gribscan",), + "imagecodecs_": ("imagecodecs-numcodecs",), +} + + +def _packages_for_codec(name: str, *, zarr_format: ZarrFormat) -> tuple[str, ...]: + """ + Names of Python packages known to provide an implementation of the codec ``name``. + + Returns an empty tuple if we don't know of any. + + Parameters + ---------- + name : str + The codec name (Zarr format 3) or codec id (Zarr format 2) we failed to resolve. + zarr_format : ZarrFormat + Which registry the codec was looked up in. + """ + if zarr_format == 2: + exact, prefixes = _NUMCODEC_PACKAGES, _NUMCODEC_PACKAGE_PREFIXES + else: + exact, prefixes = _CODEC_PACKAGES, _CODEC_PACKAGE_PREFIXES + if name in exact: + return exact[name] + for prefix, packages in prefixes.items(): + if name.startswith(prefix): + return packages + return () + + +def _missing_codec_message(name: str, *, zarr_format: ZarrFormat) -> str: + """ + Build the error message raised when no implementation of the codec ``name`` is available. + + Parameters + ---------- + name : str + The codec name (Zarr format 3) or codec id (Zarr format 2) we failed to resolve. + zarr_format : ZarrFormat + Which registry the codec was looked up in. Zarr format 2 codecs are resolved through + numcodecs, so that case points at the numcodecs registry rather than at zarr's. + """ + if zarr_format == 2: + docs_url, registry = _NUMCODECS_CODEC_DOCS_URL, "numcodecs" + else: + docs_url, registry = _ZARR_CODEC_DOCS_URL, "zarr" + msg = ( + f"An implementation for codec {name!r} is not available. Register one explicitly " + f"using the codec registry (see {docs_url}), or install a Python package that " + f"registers a codec implementation with {registry}." + ) + packages = _packages_for_codec(name, zarr_format=zarr_format) + if packages: + msg += f" Known packages supporting this codec: {', '.join(packages)}." + return msg + + +def _is_missing_numcodec_error(exc: ValueError) -> bool: + """ + Whether ``exc`` from ``numcodecs.registry.get_codec`` means "this codec isn't registered". + + numcodecs 0.15.1 added ``numcodecs.errors.UnknownCodecError`` for this. Older versions, + down to the 0.14 floor zarr supports, raise a plain ``ValueError`` instead, so fall back to + matching the message numcodecs has used throughout. + + Parameters + ---------- + exc : ValueError + The exception ``numcodecs.registry.get_codec`` raised. + """ + try: + from numcodecs.errors import UnknownCodecError as NumcodecsUnknownCodecError + except ImportError: # pragma: no cover - numcodecs < 0.15.1 + return str(exc).startswith("codec not available") + return isinstance(exc, NumcodecsUnknownCodecError) + class Registry[T](dict[str, type[T]]): def __init__(self) -> None: @@ -168,7 +295,7 @@ def get_codec_class(key: str, reload_config: bool = False) -> type[Codec]: codec_classes = _codec_registries[key] if not codec_classes: - raise KeyError(key) + raise UnknownCodecError(_missing_codec_message(key, zarr_format=3)) config_entry = config.get("codecs", {}).get(key) if config_entry is None: if len(codec_classes) == 1: @@ -179,11 +306,14 @@ def get_codec_class(key: str, reload_config: bool = False) -> type[Codec]: category=ZarrUserWarning, ) return list(codec_classes.values())[-1] - selected_codec_cls = codec_classes[config_entry] - - if selected_codec_cls: - return selected_codec_cls - raise KeyError(key) + selected_codec_cls = codec_classes.get(config_entry) + if selected_codec_cls is None: + raise UnknownCodecError( + f"Codec {key!r} is configured to use the implementation {config_entry!r}, which is " + f"not registered. Registered implementations of this codec: " + f"{sorted(codec_classes)}." + ) + return selected_codec_cls def _resolve_codec(data: dict[str, JSON]) -> Codec: @@ -321,6 +451,11 @@ def get_numcodec(data: CodecJSON_V2[str]) -> Numcodec: ------- codec : Numcodec + Raises + ------ + UnknownCodecError + If no implementation of the codec is registered with numcodecs. + Examples -------- ```python @@ -333,4 +468,13 @@ def get_numcodec(data: CodecJSON_V2[str]) -> Numcodec: from numcodecs.registry import get_codec - return get_codec(data) # type: ignore[no-any-return] + try: + return get_codec(data) # type: ignore[no-any-return] + except ValueError as e: + # Read the codec id from the input rather than from the exception: numcodecs sets its + # `codec_id` attribute to the repr of the id ("'wavpack'") rather than the id itself, + # and older numcodecs versions raise a plain ValueError that carries no id at all. + codec_id = data.get("id") + if not _is_missing_numcodec_error(e) or not isinstance(codec_id, str): + raise + raise UnknownCodecError(_missing_codec_message(codec_id, zarr_format=2)) from e diff --git a/tests/test_codecs/test_numcodecs.py b/tests/test_codecs/test_numcodecs.py index 99cd89492f..d78b73f34c 100644 --- a/tests/test_codecs/test_numcodecs.py +++ b/tests/test_codecs/test_numcodecs.py @@ -8,15 +8,10 @@ import pytest from numcodecs import GZip -try: - from numcodecs.errors import UnknownCodecError -except ImportError: - # Older versions of numcodecs don't have a separate errors module - UnknownCodecError = ValueError - from zarr import config, create_array, open_array from zarr.abc.numcodec import _is_numcodec, _is_numcodec_cls from zarr.codecs import numcodecs as _numcodecs +from zarr.errors import UnknownCodecError from zarr.registry import get_codec_class, get_numcodec if TYPE_CHECKING: @@ -297,12 +292,7 @@ def test_generic_checksum(codec_class: type[_numcodecs._NumcodecsBytesBytesCodec def test_generic_bytes_codec(codec_class: type[_numcodecs._NumcodecsArrayBytesCodec]) -> None: try: codec_class()._codec # noqa: B018 - except ValueError as e: # pragma: no cover - if "codec not available" in str(e): - pytest.xfail(f"{codec_class.codec_name} is not available: {e}") # type: ignore[misc] - else: - raise - except ImportError as e: # pragma: no cover + except (UnknownCodecError, ImportError) as e: # pragma: no cover pytest.xfail(f"{codec_class.codec_name} is not available: {e}") # type: ignore[misc] data = np.arange(0, 256, dtype="float32").reshape((16, 16)) diff --git a/tests/test_config.py b/tests/test_config.py index 47f71a798e..c22aac7603 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -24,7 +24,7 @@ from zarr.core.codec_pipeline import BatchedCodecPipeline from zarr.core.config import BadConfigError, config from zarr.core.indexing import SelectorTuple -from zarr.errors import ChunkNotFoundError, ZarrUserWarning +from zarr.errors import ChunkNotFoundError, UnknownCodecError, ZarrUserWarning from zarr.registry import ( fully_qualified_name, get_buffer_class, @@ -334,7 +334,7 @@ class NewCodec2(BytesCodec): pass # error if codec is not registered - with pytest.raises(KeyError): + with pytest.raises(UnknownCodecError): get_codec_class("missing_codec") # no warning if only one implementation is available diff --git a/tests/test_registry.py b/tests/test_registry.py new file mode 100644 index 0000000000..237ff58341 --- /dev/null +++ b/tests/test_registry.py @@ -0,0 +1,260 @@ +from __future__ import annotations + +import pytest + +from zarr.registry import ( + _CODEC_PACKAGE_PREFIXES, + _CODEC_PACKAGES, + _NUMCODEC_PACKAGE_PREFIXES, + _missing_codec_message, + _packages_for_codec, +) + + +@pytest.mark.parametrize( + ("name", "expected"), + [ + ("n5_default", ("zarr-n5",)), + ("gribberish", ("gribberish",)), + ("imagecodecs_jpeg2k", ("virtual-tiff",)), + ("omfiles.pfor", ("omfiles",)), + ("any-numcodecs.array-array", ("zarr-any-numcodecs",)), + ("totally-made-up", ()), + ], +) +def test_packages_for_codec_v3(name: str, expected: tuple[str, ...]) -> None: + """Exact names and prefixes both resolve; unknown names resolve to nothing.""" + assert _packages_for_codec(name, zarr_format=3) == expected + + +@pytest.mark.parametrize( + ("name", "expected"), + [ + ("wavpack", ("wavpack-numcodecs",)), + ("grib", ("kerchunk",)), + ("zfpy", ("numcodecs[zfpy]",)), + ("crc32c", ("numcodecs[crc32c]",)), + ("gribscan.rawgrib", ("gribscan",)), + ("imagecodecs_jpeg2k", ("imagecodecs-numcodecs",)), + ("totally-made-up", ()), + ], +) +def test_packages_for_numcodec_v2(name: str, expected: tuple[str, ...]) -> None: + """Zarr format 2 codec ids resolve against the numcodecs table.""" + assert _packages_for_codec(name, zarr_format=2) == expected + + +def test_packages_for_codec_is_format_specific() -> None: + """The same name can mean different packages in each format's registry.""" + assert _packages_for_codec("imagecodecs_jpeg2k", zarr_format=3) == ("virtual-tiff",) + assert _packages_for_codec("imagecodecs_jpeg2k", zarr_format=2) == ("imagecodecs-numcodecs",) + # `crc32c` is a codec zarr implements in format 3, so only format 2 gets a hint for it. + assert _packages_for_codec("crc32c", zarr_format=3) == () + + +def test_missing_codec_message_with_known_packages() -> None: + """The message names the codec, the docs page, and every known package.""" + msg = _missing_codec_message("n5_default", zarr_format=3) + assert "'n5_default'" in msg + assert "extending" in msg + assert "registers a codec implementation with zarr." in msg + assert "Known packages supporting this codec: zarr-n5" in msg + + +def test_missing_codec_message_without_known_packages() -> None: + """With no known package we still explain how to register one by hand.""" + msg = _missing_codec_message("totally-made-up", zarr_format=3) + assert "'totally-made-up'" in msg + assert "Known packages" not in msg + + +def test_missing_codec_message_for_zarr_format_2() -> None: + """Format 2 codecs live in the numcodecs registry, so the message must say so.""" + msg = _missing_codec_message("wavpack", zarr_format=2) + assert "numcodecs.readthedocs.io" in msg + assert "registers a codec implementation with numcodecs." in msg + assert "Known packages supporting this codec: wavpack-numcodecs" in msg + + +def test_no_prefix_shadows_another_prefix() -> None: + """First-match prefix lookup is only deterministic while no prefix contains another.""" + for table in (_CODEC_PACKAGE_PREFIXES, _NUMCODEC_PACKAGE_PREFIXES): + for a in table: + for b in table: + assert a == b or not a.startswith(b) + + +def test_mapping_does_not_shadow_builtin_codecs() -> None: + """A codec zarr implements itself must never appear in the format 3 hint table.""" + import zarr.codecs # noqa: F401 (importing registers the built-in codecs) + from zarr.registry import _codec_registries + + implemented = {name for name, reg in _codec_registries.items() if len(reg) > 0} + assert implemented, "expected importing zarr.codecs to populate the registry" + assert not (implemented & set(_CODEC_PACKAGES)) + for prefix in _CODEC_PACKAGE_PREFIXES: + assert not any(name.startswith(prefix) for name in implemented) + + +def test_get_codec_class_unknown_raises_with_package_hint( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unregistered codec with a known package names that package in the error.""" + from collections import defaultdict + + import zarr.registry + from zarr.errors import UnknownCodecError + from zarr.registry import Registry, get_codec_class + + monkeypatch.setattr(zarr.registry, "_codec_registries", defaultdict(Registry)) + with pytest.raises(UnknownCodecError, match="Known packages supporting this codec: zarr-n5"): + get_codec_class("n5_default") + + +def test_get_codec_class_unknown_raises_without_package_hint( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An unregistered codec we know nothing about still explains manual registration.""" + from collections import defaultdict + + import zarr.registry + from zarr.errors import UnknownCodecError + from zarr.registry import Registry, get_codec_class + + monkeypatch.setattr(zarr.registry, "_codec_registries", defaultdict(Registry)) + with pytest.raises(UnknownCodecError, match="An implementation for codec 'nope' is not"): + get_codec_class("nope") + + +def test_unknown_codec_error_is_exported() -> None: + """UnknownCodecError is public API now that we tell users to catch it.""" + import zarr.errors + + assert "UnknownCodecError" in zarr.errors.__all__ + + +def test_get_numcodec_unknown_raises_with_package_hint() -> None: + """A Zarr format 2 codec id we don't have names the package that provides it.""" + from zarr.errors import UnknownCodecError + from zarr.registry import get_numcodec + + with pytest.raises( + UnknownCodecError, match="Known packages supporting this codec: wavpack-numcodecs" + ): + get_numcodec({"id": "wavpack"}) + + +def test_get_numcodec_unknown_points_at_numcodecs_registry() -> None: + """The Zarr format 2 message links the numcodecs registry, not zarr's extending guide.""" + from zarr.errors import UnknownCodecError + from zarr.registry import get_numcodec + + with pytest.raises(UnknownCodecError, match="numcodecs.readthedocs.io"): + get_numcodec({"id": "definitely-not-a-real-codec"}) + + +def test_get_numcodec_known_codec_still_works() -> None: + """The happy path is untouched.""" + from numcodecs import GZip + + from zarr.registry import get_numcodec + + assert get_numcodec({"id": "gzip", "level": 2}) == GZip(level=2) # type: ignore[typeddict-unknown-key] + + +def test_get_numcodec_without_an_id_keeps_the_numcodecs_error() -> None: + """With no codec id there is nothing to look up, so numcodecs' own error stands.""" + from zarr.errors import UnknownCodecError + from zarr.registry import get_numcodec + + # Not asserting on numcodecs' exception class: it only gained a dedicated + # `numcodecs.errors.UnknownCodecError` in 0.15.1, and zarr supports numcodecs >= 0.14. + with pytest.raises(ValueError) as excinfo: + get_numcodec({"level": 2}) # type: ignore[typeddict-item,typeddict-unknown-key] + assert not isinstance(excinfo.value, UnknownCodecError) + + +async def test_open_array_with_missing_v3_codec_reports_package() -> None: + """Opening a Zarr format 3 array naming a codec we lack points at the package.""" + import json + + import zarr + from zarr.core.buffer import default_buffer_prototype + from zarr.errors import UnknownCodecError + from zarr.storage import MemoryStore + + store = MemoryStore() + metadata = { + "zarr_format": 3, + "node_type": "array", + "shape": [4], + "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [4]}}, + "chunk_key_encoding": {"name": "default"}, + "data_type": "float64", + "fill_value": 0.0, + "codecs": [{"name": "bytes"}, {"name": "n5_default"}], + "attributes": {}, + } + await store.set( + "zarr.json", + default_buffer_prototype().buffer.from_bytes(json.dumps(metadata).encode()), + ) + with pytest.raises(UnknownCodecError, match="zarr-n5"): + zarr.open_array(store=store, mode="r") + + +async def test_open_array_with_missing_v2_codec_reports_package() -> None: + """Same, for a Zarr format 2 array whose compressor id we lack.""" + import json + + import zarr + from zarr.core.buffer import default_buffer_prototype + from zarr.errors import UnknownCodecError + from zarr.storage import MemoryStore + + store = MemoryStore() + metadata = { + "zarr_format": 2, + "shape": [4], + "chunks": [4], + "dtype": " None: + """A config pinning an implementation that isn't registered must not leak a KeyError.""" + from zarr.core.config import config + from zarr.errors import UnknownCodecError + from zarr.registry import get_codec_class + + with config.set({"codecs.bytes": "some.package.RemovedBytesCodec"}): + with pytest.raises(UnknownCodecError, match="RemovedBytesCodec"): + get_codec_class("bytes") + + +def test_open_array_with_unregistered_config_pin_raises_unknown_codec_error() -> None: + """The same, through the array-open path that used to convert the KeyError for us.""" + from zarr.core.config import config + from zarr.core.metadata.v3 import parse_codecs + from zarr.errors import UnknownCodecError + + with config.set({"codecs.bytes": "some.package.RemovedBytesCodec"}): + with pytest.raises(UnknownCodecError, match="RemovedBytesCodec"): + parse_codecs([{"name": "bytes"}]) + + +def test_is_missing_numcodec_error_ignores_unrelated_value_errors() -> None: + """A ValueError about a bad configuration is not a missing codec, on any numcodecs.""" + from zarr.registry import _is_missing_numcodec_error + + assert not _is_missing_numcodec_error(ValueError("level must be between 0 and 9")) From 8a333bd7eba4d0f7181b161020e00fe676f703e1 Mon Sep 17 00:00:00 2001 From: arcusbuilds Date: Thu, 20 Aug 2026 20:21:59 +0530 Subject: [PATCH 2/3] fix: address review feedback 1. parse_codecs converts KeyError from from_dict again. The removed try/except wrapped the whole expression, not just the registry lookup, so a codec whose from_dict indexes a malformed configuration leaked a bare KeyError out of metadata parsing. On the zarr.open fallback path that KeyError was swallowed and reported as an unrelated group error: with mode="a" it surfaced as `TypeError: open_group() got an unexpected keyword argument 'shape'`. The catch is narrow, around from_dict only, since get_codec_class now raises for the lookup half. It raises MetadataValidationError naming the codec and the missing key rather than restoring the old message, which reported the missing configuration key as though it were the codec name ("Unknown codec: 'required_option'"). 2. The config-pin branch raises BadConfigError, matching get_pipeline_class, get_buffer_class and get_ndbuffer_class, which all use it for this exact situation. This also stops migrate_v3._find_numcodecs_zarr3 misreporting a config typo as a missing numcodecs codec. 3. Three tests assumed the advertised packages were absent. Both registries are entry-point driven, so they failed in any environment with zarr-n5 or wavpack-numcodecs installed, which are the packages the messages recommend. Two fixtures now remove the specific entry for the duration of the test. Verified by installing both packages and re-running. 4. test_mapping_does_not_shadow_builtin_codecs selected on "registry is non-empty", conflating loaded-in-this-process with implemented-by-zarr. It now selects on the implementing class's module, so a lazy-loaded third-party codec cannot fail it. 5. get_numcodec's Raises section notes that numcodecs' own error propagates unchanged when data carries no string "id". 6. Dropped the `pragma: no cover` on the numcodecs < 0.15.1 fallback. The min_deps env pins numcodecs==0.14.* and runs run-coverage, so that branch is measured. Also hoisted the repeated in-function imports in tests/test_registry.py to the module level. Signed-off-by: arcusbuilds --- src/zarr/core/metadata/v3.py | 12 ++- src/zarr/registry.py | 11 ++- tests/test_registry.py | 141 ++++++++++++++++++++--------------- 3 files changed, 101 insertions(+), 63 deletions(-) diff --git a/src/zarr/core/metadata/v3.py b/src/zarr/core/metadata/v3.py index a73d4aa533..988d2d369c 100644 --- a/src/zarr/core/metadata/v3.py +++ b/src/zarr/core/metadata/v3.py @@ -74,7 +74,17 @@ def parse_codecs(data: object) -> tuple[Codec, ...]: else: name_parsed, _ = parse_named_configuration(c, require_configuration=False) - out += (get_codec_class(name_parsed).from_dict(c),) + codec_cls = get_codec_class(name_parsed) + try: + out += (codec_cls.from_dict(c),) + except KeyError as e: + # A codec's `from_dict` may index its configuration directly, so a malformed + # configuration surfaces as a KeyError. Convert it: a bare KeyError escaping + # metadata parsing is swallowed by the array-then-group fallback in + # `zarr.api.asynchronous.open`, which then reports an unrelated group error. + raise MetadataValidationError( + f"Invalid configuration for codec {name_parsed!r}: missing key {e.args[0]!r}." + ) from e return out diff --git a/src/zarr/registry.py b/src/zarr/registry.py index d4ad47de7b..43ca18e30f 100644 --- a/src/zarr/registry.py +++ b/src/zarr/registry.py @@ -162,7 +162,7 @@ def _is_missing_numcodec_error(exc: ValueError) -> bool: """ try: from numcodecs.errors import UnknownCodecError as NumcodecsUnknownCodecError - except ImportError: # pragma: no cover - numcodecs < 0.15.1 + except ImportError: # numcodecs < 0.15.1 return str(exc).startswith("codec not available") return isinstance(exc, NumcodecsUnknownCodecError) @@ -308,7 +308,10 @@ def get_codec_class(key: str, reload_config: bool = False) -> type[Codec]: return list(codec_classes.values())[-1] selected_codec_cls = codec_classes.get(config_entry) if selected_codec_cls is None: - raise UnknownCodecError( + # Not UnknownCodecError: the codec is known, the implementation named in the config is + # not registered. That is a configuration problem, which is what the sibling getters in + # this module raise BadConfigError for. + raise BadConfigError( f"Codec {key!r} is configured to use the implementation {config_entry!r}, which is " f"not registered. Registered implementations of this codec: " f"{sorted(codec_classes)}." @@ -454,7 +457,9 @@ def get_numcodec(data: CodecJSON_V2[str]) -> Numcodec: Raises ------ UnknownCodecError - If no implementation of the codec is registered with numcodecs. + If no implementation of the codec id in ``data`` is registered with numcodecs. When + ``data`` carries no string ``"id"`` there is nothing to look up, and numcodecs' own + error propagates unchanged instead. Examples -------- diff --git a/tests/test_registry.py b/tests/test_registry.py index 237ff58341..4bbcbba7e3 100644 --- a/tests/test_registry.py +++ b/tests/test_registry.py @@ -1,14 +1,48 @@ from __future__ import annotations +import json + +import numcodecs.registry import pytest +import zarr +import zarr.registry +from zarr.core.buffer import default_buffer_prototype +from zarr.core.config import BadConfigError, config +from zarr.core.metadata.v3 import parse_codecs +from zarr.errors import UnknownCodecError from zarr.registry import ( _CODEC_PACKAGE_PREFIXES, _CODEC_PACKAGES, _NUMCODEC_PACKAGE_PREFIXES, + _is_missing_numcodec_error, _missing_codec_message, _packages_for_codec, + get_codec_class, + get_numcodec, ) +from zarr.storage import MemoryStore + + +@pytest.fixture +def unregistered_v3_codec(monkeypatch: pytest.MonkeyPatch) -> str: + """Guarantee a Zarr format 3 codec name resolves to nothing. + + The registry is entry-point driven, so a name the hint table advertises resolves for real + in any environment where the advertised package happens to be installed. + """ + name = "n5_default" + monkeypatch.setitem(zarr.registry._codec_registries, name, zarr.registry.Registry()) + return name + + +@pytest.fixture +def unregistered_v2_codec(monkeypatch: pytest.MonkeyPatch) -> str: + """As above, for a numcodecs codec id.""" + codec_id = "wavpack" + monkeypatch.delitem(numcodecs.registry.codec_registry, codec_id, raising=False) + monkeypatch.delitem(numcodecs.registry.entries, codec_id, raising=False) + return codec_id @pytest.mark.parametrize( @@ -89,7 +123,14 @@ def test_mapping_does_not_shadow_builtin_codecs() -> None: import zarr.codecs # noqa: F401 (importing registers the built-in codecs) from zarr.registry import _codec_registries - implemented = {name for name, reg in _codec_registries.items() if len(reg) > 0} + # Select on the implementing class's module rather than on "is this registry non-empty": + # a third-party entry-point codec lazy-loaded by an earlier test would otherwise show up + # here and fail the assertion even though it shadows nothing. + implemented = { + name + for name, reg in _codec_registries.items() + if any(cls.__module__.startswith("zarr.") for cls in reg.values()) + } assert implemented, "expected importing zarr.codecs to populate the registry" assert not (implemented & set(_CODEC_PACKAGES)) for prefix in _CODEC_PACKAGE_PREFIXES: @@ -102,11 +143,7 @@ def test_get_codec_class_unknown_raises_with_package_hint( """An unregistered codec with a known package names that package in the error.""" from collections import defaultdict - import zarr.registry - from zarr.errors import UnknownCodecError - from zarr.registry import Registry, get_codec_class - - monkeypatch.setattr(zarr.registry, "_codec_registries", defaultdict(Registry)) + monkeypatch.setattr(zarr.registry, "_codec_registries", defaultdict(zarr.registry.Registry)) with pytest.raises(UnknownCodecError, match="Known packages supporting this codec: zarr-n5"): get_codec_class("n5_default") @@ -117,11 +154,7 @@ def test_get_codec_class_unknown_raises_without_package_hint( """An unregistered codec we know nothing about still explains manual registration.""" from collections import defaultdict - import zarr.registry - from zarr.errors import UnknownCodecError - from zarr.registry import Registry, get_codec_class - - monkeypatch.setattr(zarr.registry, "_codec_registries", defaultdict(Registry)) + monkeypatch.setattr(zarr.registry, "_codec_registries", defaultdict(zarr.registry.Registry)) with pytest.raises(UnknownCodecError, match="An implementation for codec 'nope' is not"): get_codec_class("nope") @@ -133,22 +166,16 @@ def test_unknown_codec_error_is_exported() -> None: assert "UnknownCodecError" in zarr.errors.__all__ -def test_get_numcodec_unknown_raises_with_package_hint() -> None: +def test_get_numcodec_unknown_raises_with_package_hint(unregistered_v2_codec: str) -> None: """A Zarr format 2 codec id we don't have names the package that provides it.""" - from zarr.errors import UnknownCodecError - from zarr.registry import get_numcodec - with pytest.raises( UnknownCodecError, match="Known packages supporting this codec: wavpack-numcodecs" ): - get_numcodec({"id": "wavpack"}) + get_numcodec({"id": unregistered_v2_codec}) def test_get_numcodec_unknown_points_at_numcodecs_registry() -> None: """The Zarr format 2 message links the numcodecs registry, not zarr's extending guide.""" - from zarr.errors import UnknownCodecError - from zarr.registry import get_numcodec - with pytest.raises(UnknownCodecError, match="numcodecs.readthedocs.io"): get_numcodec({"id": "definitely-not-a-real-codec"}) @@ -157,16 +184,11 @@ def test_get_numcodec_known_codec_still_works() -> None: """The happy path is untouched.""" from numcodecs import GZip - from zarr.registry import get_numcodec - assert get_numcodec({"id": "gzip", "level": 2}) == GZip(level=2) # type: ignore[typeddict-unknown-key] def test_get_numcodec_without_an_id_keeps_the_numcodecs_error() -> None: """With no codec id there is nothing to look up, so numcodecs' own error stands.""" - from zarr.errors import UnknownCodecError - from zarr.registry import get_numcodec - # Not asserting on numcodecs' exception class: it only gained a dedicated # `numcodecs.errors.UnknownCodecError` in 0.15.1, and zarr supports numcodecs >= 0.14. with pytest.raises(ValueError) as excinfo: @@ -174,15 +196,15 @@ def test_get_numcodec_without_an_id_keeps_the_numcodecs_error() -> None: assert not isinstance(excinfo.value, UnknownCodecError) -async def test_open_array_with_missing_v3_codec_reports_package() -> None: - """Opening a Zarr format 3 array naming a codec we lack points at the package.""" - import json +def test_is_missing_numcodec_error_ignores_unrelated_value_errors() -> None: + """A ValueError about a bad configuration is not a missing codec, on any numcodecs.""" + assert not _is_missing_numcodec_error(ValueError("level must be between 0 and 9")) - import zarr - from zarr.core.buffer import default_buffer_prototype - from zarr.errors import UnknownCodecError - from zarr.storage import MemoryStore +async def test_open_array_with_missing_v3_codec_reports_package( + unregistered_v3_codec: str, +) -> None: + """Opening a Zarr format 3 array naming a codec we lack points at the package.""" store = MemoryStore() metadata = { "zarr_format": 3, @@ -192,7 +214,7 @@ async def test_open_array_with_missing_v3_codec_reports_package() -> None: "chunk_key_encoding": {"name": "default"}, "data_type": "float64", "fill_value": 0.0, - "codecs": [{"name": "bytes"}, {"name": "n5_default"}], + "codecs": [{"name": "bytes"}, {"name": unregistered_v3_codec}], "attributes": {}, } await store.set( @@ -203,22 +225,17 @@ async def test_open_array_with_missing_v3_codec_reports_package() -> None: zarr.open_array(store=store, mode="r") -async def test_open_array_with_missing_v2_codec_reports_package() -> None: +async def test_open_array_with_missing_v2_codec_reports_package( + unregistered_v2_codec: str, +) -> None: """Same, for a Zarr format 2 array whose compressor id we lack.""" - import json - - import zarr - from zarr.core.buffer import default_buffer_prototype - from zarr.errors import UnknownCodecError - from zarr.storage import MemoryStore - store = MemoryStore() metadata = { "zarr_format": 2, "shape": [4], "chunks": [4], "dtype": " None: zarr.open_array(store=store, mode="r") -def test_get_codec_class_with_unregistered_config_pin_raises_unknown_codec_error() -> None: - """A config pinning an implementation that isn't registered must not leak a KeyError.""" - from zarr.core.config import config - from zarr.errors import UnknownCodecError - from zarr.registry import get_codec_class - +def test_get_codec_class_with_unregistered_config_pin_raises_bad_config_error() -> None: + """A config pinning an implementation that isn't registered is a config error.""" with config.set({"codecs.bytes": "some.package.RemovedBytesCodec"}): - with pytest.raises(UnknownCodecError, match="RemovedBytesCodec"): + with pytest.raises(BadConfigError, match="RemovedBytesCodec"): get_codec_class("bytes") -def test_open_array_with_unregistered_config_pin_raises_unknown_codec_error() -> None: - """The same, through the array-open path that used to convert the KeyError for us.""" - from zarr.core.config import config - from zarr.core.metadata.v3 import parse_codecs - from zarr.errors import UnknownCodecError - +def test_parse_codecs_with_unregistered_config_pin_raises_bad_config_error() -> None: + """The same, through the array-open path, and never as a bare KeyError.""" with config.set({"codecs.bytes": "some.package.RemovedBytesCodec"}): - with pytest.raises(UnknownCodecError, match="RemovedBytesCodec"): + with pytest.raises(BadConfigError, match="RemovedBytesCodec"): parse_codecs([{"name": "bytes"}]) -def test_is_missing_numcodec_error_ignores_unrelated_value_errors() -> None: - """A ValueError about a bad configuration is not a missing codec, on any numcodecs.""" - from zarr.registry import _is_missing_numcodec_error +def test_parse_codecs_converts_keyerror_from_from_dict() -> None: + """A codec whose from_dict indexes a malformed config must not leak a KeyError. - assert not _is_missing_numcodec_error(ValueError("level must be between 0 and 9")) + A bare KeyError out of metadata parsing is caught by the array-then-group fallback in + `zarr.api.asynchronous.open`, which then reports an unrelated group error. + """ + from zarr.codecs import BytesCodec + from zarr.errors import MetadataValidationError + from zarr.registry import register_codec + + class PickyCodec(BytesCodec): + @classmethod + def from_dict(cls, data: object) -> PickyCodec: + data["configuration"]["required_option"] # type: ignore[index] + return cls() + + register_codec("test_picky", PickyCodec) + with pytest.raises(MetadataValidationError, match="required_option"): + parse_codecs([{"name": "test_picky", "configuration": {}}]) From e5ac63e6e4e96141cd34740f9dfee7904f2f1b97 Mon Sep 17 00:00:00 2001 From: arcusbuilds Date: Thu, 20 Aug 2026 22:31:57 +0530 Subject: [PATCH 3/3] fix: address the second review round Four defects, all found by review after the previous round was reported clean. get_numcodec no longer wraps the numcodecs call in an exception handler. Catching cannot distinguish "this id is unregistered" from "a registered codec rejected its configuration" or "a wrapper codec failed to resolve an inner codec", and it was relabelling both of the latter with the outer id plus a package hint that was wrong. Reproduced: a wrapper registered as `wavpack` whose from_config resolved a missing inner codec reported "An implementation for codec 'wavpack' is not available ... install wavpack-numcodecs", when wavpack was installed and the missing codec was something else entirely. It now performs the lookups numcodecs performs, before delegating. That also fixes reading `id` off a non-mapping input, which raised AttributeError where a ValueError used to propagate. And it removes _is_missing_numcodec_error, and with it the numcodecs <0.15.1 compatibility branch, since there is no longer an exception to classify. Note this narrows the zarr error to Mapping inputs; a duck-typed mapping now gets numcodecs' error instead, as it did before this PR. The imagecodecs_ prefix in the Zarr format 3 table pointed at virtual-tiff, which declares 15 of the 81 imagecodecs_* names under zarr.codecs; imagecodecs-numcodecs declares all 81, but under numcodecs.codecs. Users of the other 66 names were told to install a package that does not provide them. The format 3 side now lists the 15 exact names, so the rest get no hint rather than a wrong one. The format 2 side keeps the prefix, where it is correct. test_parse_codecs_converts_keyerror_from_from_dict leaked test_picky into the global codec registry, in the file that also reads that global. Tests: mutation testing showed six mutations surviving. Added the missing coverage for a registered codec rejecting its configuration, wrapper codecs resolving an inner codec by either route, non-mapping input, the imagecodecs_ over-match, and the _resolve_codec entry point. Message assertions now pin the whole string and the URL constants rather than substrings, which had allowed both documentation URLs to be replaced with wrong ones and half the message body to be deleted with every test still passing. Also corrects the docs path to src/zarr/registry.py, and rewrites the changelog to lead with the exception-type changes and to document the parse_codecs change it had omitted. Signed-off-by: arcusbuilds --- changes/4277.feature.md | 40 +++++++---- docs/user-guide/extending.md | 2 +- src/zarr/registry.py | 79 +++++++++++----------- tests/test_registry.py | 125 ++++++++++++++++++++++++++++++----- 4 files changed, 177 insertions(+), 69 deletions(-) diff --git a/changes/4277.feature.md b/changes/4277.feature.md index 03caaadce3..f5c247496d 100644 --- a/changes/4277.feature.md +++ b/changes/4277.feature.md @@ -1,14 +1,28 @@ -When Zarr cannot find an implementation for a codec, the error now names Python packages known to -provide that codec, e.g. `An implementation for codec 'wavpack' is not available. Register one -explicitly using the codec registry (see ...), or install a Python package that registers a codec -implementation with numcodecs. Known packages supporting this codec: wavpack-numcodecs.` This -covers codecs that `numcodecs` itself gates behind an optional dependency — `zfpy`, `pcodec`, -`crc32c` and `msgpack2` now point at the matching `numcodecs[...]` extra — as well as third-party -packages. Codec authors can add their published package to the table in `zarr/registry.py`. - -`zarr.registry.get_codec_class` now raises `zarr.errors.UnknownCodecError` instead of `KeyError`, -both for a codec with no registered implementation and for a codec whose configured -implementation is not registered. `zarr.registry.get_numcodec` raises it instead of the -`ValueError` (`numcodecs.errors.UnknownCodecError` on numcodecs 0.15.1 and later) that numcodecs -raises for an unregistered Zarr format 2 codec id. All of these are subclasses of `ValueError`. +`zarr.registry.get_codec_class` now raises `zarr.errors.UnknownCodecError` instead of `KeyError` +when no implementation is registered for a codec, and `zarr.core.config.BadConfigError` instead of +`KeyError` when the implementation named in `config["codecs"][name]` is not registered. +`zarr.registry.get_numcodec` raises `UnknownCodecError` instead of the `ValueError` numcodecs +raises for an unregistered Zarr format 2 codec id (`numcodecs.errors.UnknownCodecError` on +numcodecs 0.15.1 and later). All of these are subclasses of `ValueError`, so `except ValueError` +is unaffected, but `except KeyError` and `except numcodecs.errors.UnknownCodecError` are. + +These errors now name Python packages known to provide the codec, so that a user who cannot read +an array learns what to install: + +``` +An implementation for codec 'wavpack' is not available. Register one explicitly using the codec +registry (see ...), or install a Python package that registers a codec implementation with +numcodecs. Known packages supporting this codec: wavpack-numcodecs. +``` + +The tables covering this live in `src/zarr/registry.py`, one per Zarr format, and include the +codecs `numcodecs` gates behind its own optional dependencies (`zfpy`, `pcodec`, `crc32c`, +`msgpack2`). Codec authors can add their published package to them. + +A codec whose `from_dict` raises `KeyError` on a malformed configuration now surfaces as +`zarr.errors.MetadataValidationError` naming the codec and the missing key. Previously it was +reported as `UnknownCodecError: Unknown codec: ''`, presenting a configuration +key as though it were a codec name, and on the `zarr.open` path a bare `KeyError` could be +swallowed by the array-then-group fallback and reported as an unrelated group error. + `zarr.errors.UnknownCodecError` is now exported from `zarr.errors`. diff --git a/docs/user-guide/extending.md b/docs/user-guide/extending.md index 6be98ca4e5..507afedea7 100644 --- a/docs/user-guide/extending.md +++ b/docs/user-guide/extending.md @@ -67,7 +67,7 @@ If someone opens an array that uses your codec without your package installed, Z [`zarr.errors.UnknownCodecError`][] explaining how to register an implementation. Zarr also keeps a small table of codec names and the published packages that provide them, and names those packages in that error. Once your package is on PyPI, please open a pull request adding -it to the codec-package table in `zarr/registry.py`, so that users get a message telling them +it to the codec-package tables in `src/zarr/registry.py`, so that users get a message telling them exactly what to install. !!! note diff --git a/src/zarr/registry.py b/src/zarr/registry.py index 43ca18e30f..a7537e5023 100644 --- a/src/zarr/registry.py +++ b/src/zarr/registry.py @@ -2,6 +2,7 @@ import warnings from collections import defaultdict +from collections.abc import Mapping from importlib.metadata import entry_points as get_entry_points from typing import TYPE_CHECKING, Any @@ -52,12 +53,31 @@ # # The two Zarr formats resolve codecs through different registries, so they get different # tables: a name can mean one thing as a Zarr format 3 codec name and another as a Zarr -# format 2 codec id. `imagecodecs_*` is exactly that -- `virtual-tiff` declares those names -# under `zarr.codecs`, while `imagecodecs-numcodecs` declares them under `numcodecs.codecs`. +# format 2 codec id. `imagecodecs_*` is exactly that -- `virtual-tiff` declares 15 of those +# names under `zarr.codecs`, while `imagecodecs-numcodecs` declares all 81 under +# `numcodecs.codecs`, so the format 2 side can use a prefix and the format 3 side cannot. # Zarr format 3 codec names (entry point group "zarr.codecs"). _CODEC_PACKAGES: dict[str, tuple[str, ...]] = { "gribberish": ("gribberish",), + # `virtual-tiff` declares these 15 `imagecodecs_*` names, out of the 81 that exist as + # numcodecs ids. They are listed exactly rather than by prefix so that the other 66 get no + # hint instead of a hint pointing at a package that does not provide them. + "imagecodecs_deflate": ("virtual-tiff",), + "imagecodecs_delta": ("virtual-tiff",), + "imagecodecs_floatpred": ("virtual-tiff",), + "imagecodecs_jetraw": ("virtual-tiff",), + "imagecodecs_jpeg": ("virtual-tiff",), + "imagecodecs_jpeg2k": ("virtual-tiff",), + "imagecodecs_jpeg8": ("virtual-tiff",), + "imagecodecs_jpegxl": ("virtual-tiff",), + "imagecodecs_jpegxr": ("virtual-tiff",), + "imagecodecs_lerc": ("virtual-tiff",), + "imagecodecs_lzw": ("virtual-tiff",), + "imagecodecs_packbits": ("virtual-tiff",), + "imagecodecs_png": ("virtual-tiff",), + "imagecodecs_webp": ("virtual-tiff",), + "imagecodecs_zstd": ("virtual-tiff",), "n5_default": ("zarr-n5",), } @@ -65,7 +85,6 @@ # that provide many codecs namespace them behind a shared prefix, so one entry covers them all. _CODEC_PACKAGE_PREFIXES: dict[str, tuple[str, ...]] = { "any-numcodecs.": ("zarr-any-numcodecs",), - "imagecodecs_": ("virtual-tiff",), "omfiles.": ("omfiles",), "virtual_tiff.": ("virtual-tiff",), } @@ -147,26 +166,6 @@ def _missing_codec_message(name: str, *, zarr_format: ZarrFormat) -> str: return msg -def _is_missing_numcodec_error(exc: ValueError) -> bool: - """ - Whether ``exc`` from ``numcodecs.registry.get_codec`` means "this codec isn't registered". - - numcodecs 0.15.1 added ``numcodecs.errors.UnknownCodecError`` for this. Older versions, - down to the 0.14 floor zarr supports, raise a plain ``ValueError`` instead, so fall back to - matching the message numcodecs has used throughout. - - Parameters - ---------- - exc : ValueError - The exception ``numcodecs.registry.get_codec`` raised. - """ - try: - from numcodecs.errors import UnknownCodecError as NumcodecsUnknownCodecError - except ImportError: # numcodecs < 0.15.1 - return str(exc).startswith("codec not available") - return isinstance(exc, NumcodecsUnknownCodecError) - - class Registry[T](dict[str, type[T]]): def __init__(self) -> None: super().__init__() @@ -457,9 +456,9 @@ def get_numcodec(data: CodecJSON_V2[str]) -> Numcodec: Raises ------ UnknownCodecError - If no implementation of the codec id in ``data`` is registered with numcodecs. When - ``data`` carries no string ``"id"`` there is nothing to look up, and numcodecs' own - error propagates unchanged instead. + If ``data`` carries a string ``"id"`` that is not registered with numcodecs. Any other + failure, including a registered codec rejecting its configuration and a ``data`` that is + not a mapping, propagates from numcodecs unchanged. Examples -------- @@ -471,15 +470,19 @@ def get_numcodec(data: CodecJSON_V2[str]) -> Numcodec: ``` """ - from numcodecs.registry import get_codec - - try: - return get_codec(data) # type: ignore[no-any-return] - except ValueError as e: - # Read the codec id from the input rather than from the exception: numcodecs sets its - # `codec_id` attribute to the repr of the id ("'wavpack'") rather than the id itself, - # and older numcodecs versions raise a plain ValueError that carries no id at all. - codec_id = data.get("id") - if not _is_missing_numcodec_error(e) or not isinstance(codec_id, str): - raise - raise UnknownCodecError(_missing_codec_message(codec_id, zarr_format=2)) from e + from numcodecs.registry import codec_registry, entries, get_codec + + # Check whether numcodecs can resolve the id *before* handing off, rather than catching what + # `get_codec` raises. Catching cannot tell "this id is unregistered" from "a registered codec + # rejected its configuration" or from "a wrapper codec failed to resolve an inner codec", and + # relabelling either of those with this id would attach a package hint that is simply wrong. + # This mirrors the two lookups `get_codec` performs (it then tests the result for + # truthiness rather than membership, which only differs for a falsy registry value). + # Widened to `object` deliberately: `data` is annotated as a TypedDict, but this is a public + # function and callers pass whatever they like. numcodecs coerces with `dict(config)` and + # raises for anything that is not a mapping, which is the behaviour to preserve. + raw: object = data + codec_id = raw.get("id") if isinstance(raw, Mapping) else None + if isinstance(codec_id, str) and codec_id not in codec_registry and codec_id not in entries: + raise UnknownCodecError(_missing_codec_message(codec_id, zarr_format=2)) + return get_codec(data) # type: ignore[no-any-return] diff --git a/tests/test_registry.py b/tests/test_registry.py index 4bbcbba7e3..767d3e0439 100644 --- a/tests/test_registry.py +++ b/tests/test_registry.py @@ -15,7 +15,8 @@ _CODEC_PACKAGE_PREFIXES, _CODEC_PACKAGES, _NUMCODEC_PACKAGE_PREFIXES, - _is_missing_numcodec_error, + _NUMCODECS_CODEC_DOCS_URL, + _ZARR_CODEC_DOCS_URL, _missing_codec_message, _packages_for_codec, get_codec_class, @@ -89,25 +90,34 @@ def test_packages_for_codec_is_format_specific() -> None: def test_missing_codec_message_with_known_packages() -> None: """The message names the codec, the docs page, and every known package.""" msg = _missing_codec_message("n5_default", zarr_format=3) - assert "'n5_default'" in msg - assert "extending" in msg - assert "registers a codec implementation with zarr." in msg - assert "Known packages supporting this codec: zarr-n5" in msg + assert msg == ( + "An implementation for codec 'n5_default' is not available. Register one explicitly " + f"using the codec registry (see {_ZARR_CODEC_DOCS_URL}), or install a Python package " + "that registers a codec implementation with zarr. Known packages supporting this " + "codec: zarr-n5." + ) def test_missing_codec_message_without_known_packages() -> None: """With no known package we still explain how to register one by hand.""" msg = _missing_codec_message("totally-made-up", zarr_format=3) - assert "'totally-made-up'" in msg + assert msg == ( + "An implementation for codec 'totally-made-up' is not available. Register one " + f"explicitly using the codec registry (see {_ZARR_CODEC_DOCS_URL}), or install a " + "Python package that registers a codec implementation with zarr." + ) assert "Known packages" not in msg def test_missing_codec_message_for_zarr_format_2() -> None: """Format 2 codecs live in the numcodecs registry, so the message must say so.""" msg = _missing_codec_message("wavpack", zarr_format=2) - assert "numcodecs.readthedocs.io" in msg - assert "registers a codec implementation with numcodecs." in msg - assert "Known packages supporting this codec: wavpack-numcodecs" in msg + assert msg == ( + "An implementation for codec 'wavpack' is not available. Register one explicitly " + f"using the codec registry (see {_NUMCODECS_CODEC_DOCS_URL}), or install a Python " + "package that registers a codec implementation with numcodecs. Known packages " + "supporting this codec: wavpack-numcodecs." + ) def test_no_prefix_shadows_another_prefix() -> None: @@ -176,8 +186,10 @@ def test_get_numcodec_unknown_raises_with_package_hint(unregistered_v2_codec: st def test_get_numcodec_unknown_points_at_numcodecs_registry() -> None: """The Zarr format 2 message links the numcodecs registry, not zarr's extending guide.""" - with pytest.raises(UnknownCodecError, match="numcodecs.readthedocs.io"): + with pytest.raises(UnknownCodecError) as excinfo: get_numcodec({"id": "definitely-not-a-real-codec"}) + assert _NUMCODECS_CODEC_DOCS_URL in str(excinfo.value) + assert _ZARR_CODEC_DOCS_URL not in str(excinfo.value) def test_get_numcodec_known_codec_still_works() -> None: @@ -196,9 +208,85 @@ def test_get_numcodec_without_an_id_keeps_the_numcodecs_error() -> None: assert not isinstance(excinfo.value, UnknownCodecError) -def test_is_missing_numcodec_error_ignores_unrelated_value_errors() -> None: - """A ValueError about a bad configuration is not a missing codec, on any numcodecs.""" - assert not _is_missing_numcodec_error(ValueError("level must be between 0 and 9")) +def test_get_numcodec_does_not_relabel_a_bad_configuration() -> None: + """A registered codec rejecting its config is not a missing codec. + + Telling the user to install a package they already have would be the same misleading + error this module exists to remove, pointing the other way. + """ + with pytest.raises(ValueError) as excinfo: + get_numcodec({"id": "bitround", "keepbits": -1}) # type: ignore[typeddict-unknown-key] + assert not isinstance(excinfo.value, UnknownCodecError) + assert str(excinfo.value) == "keepbits must be zero or positive" + + +@pytest.mark.parametrize("via", ["zarr", "numcodecs"]) +def test_get_numcodec_does_not_relabel_a_missing_inner_codec(via: str) -> None: + """A wrapper codec failing on an inner codec must not be blamed on the outer id. + + The outer id is registered, so relabelling it would advertise a package the user has + already installed. Covers both routes a real wrapper takes to resolve its inner codec: + zarr's ``get_numcodec`` and numcodecs' own ``get_codec``. + """ + import numcodecs.registry + from numcodecs.abc import Codec + + resolve = get_numcodec if via == "zarr" else numcodecs.registry.get_codec + + class WrapperCodec(Codec): # type: ignore[misc] + codec_id = "test_wrapper" + + @classmethod + def from_config(cls, config: dict[str, object]) -> WrapperCodec: + resolve({"id": "some-missing-inner-codec"}) + return cls() + + def encode(self, buf: object) -> object: + return buf + + def decode(self, buf: object, out: object = None) -> object: + return buf + + numcodecs.registry.register_codec(WrapperCodec, codec_id="test_wrapper") + try: + with pytest.raises(ValueError) as excinfo: + get_numcodec({"id": "test_wrapper"}) + assert "some-missing-inner-codec" in str(excinfo.value) + assert "test_wrapper" not in str(excinfo.value) + finally: + numcodecs.registry.codec_registry.pop("test_wrapper", None) + + +@pytest.mark.parametrize("data", ["abc", ["ab"]]) +def test_get_numcodec_non_mapping_input_still_raises_value_error(data: object) -> None: + """Input that is not a mapping must reach numcodecs rather than raising AttributeError. + + Reading ``id`` off the input unconditionally turned numcodecs' ValueError into an + AttributeError, which is not a ValueError and so escaped handlers that caught it. Both + params end in a ValueError from numcodecs, by different routes: ``"abc"`` fails + ``dict(config)`` coercion, while ``["ab"]`` coerces to ``{"a": "b"}`` and then has no id. + """ + with pytest.raises(ValueError): + get_numcodec(data) # type: ignore[arg-type] + + +def test_imagecodecs_prefix_does_not_over_match_in_zarr_format_3() -> None: + """virtual-tiff provides 15 of the 81 `imagecodecs_*` names; the rest must get no hint. + + Recommending virtual-tiff for a name it does not provide is worse than saying nothing. + """ + assert _packages_for_codec("imagecodecs_jpeg2k", zarr_format=3) == ("virtual-tiff",) + for name in ("imagecodecs_jpegls", "imagecodecs_avif", "imagecodecs_blosc"): + assert _packages_for_codec(name, zarr_format=3) == () + assert _packages_for_codec(name, zarr_format=2) == ("imagecodecs-numcodecs",) + + +def test_resolve_codec_reports_missing_codec() -> None: + """The other public entry point into `get_codec_class` gets the message too.""" + import zarr + + with pytest.raises(UnknownCodecError, match="Known packages supporting this codec: zarr-n5"): + zarr.create_array({}, shape=(4,), dtype="uint8", compressors=[{"name": "n5_default"}]) async def test_open_array_with_missing_v3_codec_reports_package( @@ -221,7 +309,7 @@ async def test_open_array_with_missing_v3_codec_reports_package( "zarr.json", default_buffer_prototype().buffer.from_bytes(json.dumps(metadata).encode()), ) - with pytest.raises(UnknownCodecError, match="zarr-n5"): + with pytest.raises(UnknownCodecError, match="Known packages supporting this codec: zarr-n5"): zarr.open_array(store=store, mode="r") @@ -244,7 +332,9 @@ async def test_open_array_with_missing_v2_codec_reports_package( ".zarray", default_buffer_prototype().buffer.from_bytes(json.dumps(metadata).encode()), ) - with pytest.raises(UnknownCodecError, match="wavpack-numcodecs"): + with pytest.raises( + UnknownCodecError, match="Known packages supporting this codec: wavpack-numcodecs" + ): zarr.open_array(store=store, mode="r") @@ -262,7 +352,7 @@ def test_parse_codecs_with_unregistered_config_pin_raises_bad_config_error() -> parse_codecs([{"name": "bytes"}]) -def test_parse_codecs_converts_keyerror_from_from_dict() -> None: +def test_parse_codecs_converts_keyerror_from_from_dict(monkeypatch: pytest.MonkeyPatch) -> None: """A codec whose from_dict indexes a malformed config must not leak a KeyError. A bare KeyError out of metadata parsing is caught by the array-then-group fallback in @@ -278,6 +368,7 @@ def from_dict(cls, data: object) -> PickyCodec: data["configuration"]["required_option"] # type: ignore[index] return cls() + monkeypatch.setitem(zarr.registry._codec_registries, "test_picky", zarr.registry.Registry()) register_codec("test_picky", PickyCodec) - with pytest.raises(MetadataValidationError, match="required_option"): + with pytest.raises(MetadataValidationError, match="test_picky.*required_option"): parse_codecs([{"name": "test_picky", "configuration": {}}])