Skip to content

fix: preserve Numcodecs filter shape and decode order - #4558

Open
Don-Yin wants to merge 1 commit into
zarr-developers:mainfrom
Don-Yin:fix-numcodecs-filter-shape
Open

Don-Yin wants to merge 1 commit into
zarr-developers:mainfrom
Don-Yin:fix-numcodecs-filter-shape

Conversation

@Don-Yin

@Don-Yin Don-Yin commented Oct 22, 2026

Copy link
Copy Markdown

Fixes #4553.

Some Numcodecs filters (e.g. PackBits) return a flat array, so a following TransposeCodec fails on write. This reshapes the encoded output to the shape from resolve_metadata.

Reshaping alone isn't enough: after undoing the transpose the array can be F-ordered, and Numcodecs decodes in memory order, so Delta and FixedScaleOffset round-trips come back scrambled. Making the input C-contiguous before decoding fixes that.

Builds on @ghuls's draft in the issue. The existing filter round-trip test now also runs with a following transpose.

Filter round-trip results

  • I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions (n/a)
  • New/modified features documented in docs/user-guide/*.md (n/a)
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

Restore the shape declared by the filter and normalize decode input to
C order after a transpose. Extend the existing round-trip cases to cover
a following TransposeCodec without relaxing exact array equality.

The shape-restoration approach follows ghuls's draft in zarr-developers#4553.

Signed-off-by: Don Yin <Don_Yin@outlook.com>
@codecov

codecov Bot commented Oct 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.71%. Comparing base (957a5f6) to head (b825666).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4558   +/-   ##
=======================================
  Coverage   94.71%   94.71%           
=======================================
  Files          94       94           
  Lines       13663    13664    +1     
=======================================
+ Hits        12941    12942    +1     
  Misses        722      722           
Files with missing lines Coverage Δ
src/zarr/codecs/numcodecs/_codecs.py 95.50% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ghuls

ghuls commented Oct 23, 2026

Copy link
Copy Markdown

Thanks for fixing it properly.

This branch has not been deployed

No deployments
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.

_NumcodecsArrayArrayCodec._encode_sync does not reshape the data on encode

2 participants