feat: name packages that provide a codec zarr cannot find - #4277
feat: name packages that provide a codec zarr cannot find#4277arcusbuilds wants to merge 4 commits into
Conversation
e96b224 to
4e39185
Compare
Closes zarr-developers#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 <docs>), 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 <srijankeshri007@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4277 +/- ##
==========================================
+ Coverage 94.12% 94.14% +0.02%
==========================================
Files 92 92
Lines 12831 12863 +32
==========================================
+ Hits 12077 12110 +33
+ Misses 754 753 -1
🚀 New features to boost your workflow:
|
|
this looks good, I had claude look for issues and it found some. Here's the summary: 🤖 AI text below 🤖 Review notes on this PR. Overall the feature is solid: the happy paths gain no overhead, the per-format table split is justified by the real 1. 2. The config-pin branch raises the wrong exception type. 3. Three tests depend on the hinted packages being absent. 4. 5. Minor: 6. Minor: the Nits, take or leave: the new test file re-imports the same names ( On the hand-maintained tables themselves: the pragmatic call is defensible (entry-point discovery is impossible for an uninstalled package), but the rows are unverifiable by CI and uncorrectable in shipped releases — if a listed name is ever abandoned and re-claimed on PyPI, released zarr error text keeps recommending it. Pointing the message at a docs page listing known codec packages would trade some in-terminal actionability for retroactive correctability; either choice works as long as it's made deliberately. |
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 <srijankeshri007@gmail.com>
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 <srijankeshri007@gmail.com>
2 participants
Closes #4271.
When zarr cannot resolve a codec, the error repeats the name the user already has:
It does not say the failure is fixable, or which package fixes it. Since zarr and numcodecs both load codecs from entry points,
pip install <package>is the whole fix, so naming the package is usually enough. Discussion #4269 is the case that prompted this.Two tables in
zarr/registry.pynow map codec names to the packages that provide them:There are two tables because the formats use different registries and a name can mean different things in each.
imagecodecs_*isvirtual-tiffunderzarr.codecsbutimagecodecs-numcodecsundernumcodecs.codecs.crc32cis built in for format 3 and needsnumcodecs[crc32c]for format 2. Entries come from each package's declared entry points, and every distribution named resolves on PyPI. The tables also cover numcodecs' own optional extras (zfpy,pcodec,crc32c,msgpack2), which are the most common case in practice.Backwards compatibility
get_codec_classraisesUnknownCodecErrorinstead ofKeyError, both for an unregistered codec and for a codec whose configured implementation is not registered.get_numcodecraises it instead of theValueErrornumcodecs raises. All areValueErrorsubclasses, soexcept ValueErroris unaffected.A
KeyErrorcannot carry the message: its__str__reprs the argument, so multi-sentence text comes back quoted and escaped. If you would rather keep the exception types, the tables and the message builder stand on their own and I can cut the rest.numcodecs.errorsonly exists from 0.15.1 while the declared floor is 0.14, so the unregistered-codec check prefers that exception type where it can be imported and falls back to matching the message.Testing
New
tests/test_registry.pycovers the tables, the message for both formats, bothget_codec_classfailure branches,get_numcodec, and two cases that open an array whose metadata names a missing codec. Full suite passes on numcodecs 0.16.5, and the files touched here also pass againstnumcodecs==0.14.1.