Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor UX improvement: Add the UnknownCodecError. #689

Merged
merged 11 commits into from
Jan 17, 2025

Conversation

cwognum
Copy link
Contributor

@cwognum cwognum commented Jan 14, 2025

See zarr-developers/zarr-python#2508 for context.

Since numcodecs supports registering custom codecs in downstream libraries, there can be situations where the environment in which data was encoded is not the same as the environment in which the data is decoded (e.g. opening a Zarr archive that was created in a different environment). This can lead to missing codecs.

It would be a nice UX improvement if you catch that error and can offer codec specific solutions.

This change would enable something like:

import numcodecs

try: 
    numcodecs.get_codec({"codec_id": "imagecodecs_jpeg2k"})
except UnknownCodecError as error:
    if error.codec_id == "imagecodecs_jpeg2k":
        raise RuntimeError("This Zarr archive requires `imagecodecs`, to install it use ...") from error

TODO:

  • Unit tests and/or doctests in docstrings
  • Tests pass locally
  • Docstrings and API docs for any new/modified user-facing classes and functions
  • Changes documented in docs/release.rst
  • Docs build locally
  • GitHub Actions CI passes
  • Test coverage to 100% (Codecov passes)

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.70%. Comparing base (7ed1ace) to head (e4dac91).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #689   +/-   ##
=======================================
  Coverage   99.70%   99.70%           
=======================================
  Files          62       63    +1     
  Lines        2749     2755    +6     
=======================================
+ Hits         2741     2747    +6     
  Misses          8        8           
Files with missing lines Coverage Δ
numcodecs/errors.py 100.00% <100.00%> (ø)
numcodecs/registry.py 100.00% <100.00%> (ø)
numcodecs/tests/test_registry.py 100.00% <100.00%> (ø)

Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very 👍 doing this, thanks for the PR. I left a suggestion for the test, and also I'm a bit on the fence about ValueError, so let me know your thoughts there.

@cwognum
Copy link
Contributor Author

cwognum commented Jan 17, 2025

@d-v-b @dstansby Ready for another round of review!

@dstansby dstansby enabled auto-merge (squash) January 17, 2025 18:56
@dstansby dstansby merged commit 6d090a3 into zarr-developers:main Jan 17, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants