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 2 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 12830 12869 +39
==========================================
+ Hits 12076 12116 +40
+ 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>
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.