Skip to content

Change compressor (zarr v2) to compressors (zarr v3)#1657

Merged
leewujung merged 2 commits intoOSOceanAcoustics:mainfrom
leewujung:zarr_v3_compressors
Apr 19, 2026
Merged

Change compressor (zarr v2) to compressors (zarr v3)#1657
leewujung merged 2 commits intoOSOceanAcoustics:mainfrom
leewujung:zarr_v3_compressors

Conversation

@leewujung
Copy link
Copy Markdown
Member

This is a bug surfaced in running the krill_freq_diff.pynb notebook on echopype-examples: OSOceanAcoustics/echopype-examples#92

It turns out Zarr v3 supports condec chaining so instead of compressor, it uses compressors (plural)! The example is given in the Zarr documentation: https://docs.xarray.dev/en/stable/user-guide/io.html#zarr-compressors-and-filters

I also found this test is not doing anything. It might be something that was functioning but then something got changed so the test is no longer needed -- @LOCEANlloydizard if you have a moment could you confirm I am not mistaking, that the lazy_encodings is indeed empty? If so, maybe we drop it?

@ctuguinay : I think this is why your temp fix to do compress=False worked in OSOceanAcoustics/echopype-examples#89, but it's not the root of the problem.

@leewujung leewujung added bug Something isn't working dependencies Anything related to dependencies labels Apr 18, 2026
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.75%. Comparing base (6cf6cee) to head (2cfb4fb).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1657      +/-   ##
==========================================
+ Coverage   85.58%   85.75%   +0.17%     
==========================================
  Files          79       78       -1     
  Lines        6998     7040      +42     
==========================================
+ Hits         5989     6037      +48     
+ Misses       1009     1003       -6     
Flag Coverage Δ
integration 80.98% <ø> (+0.34%) ⬆️
unit 60.29% <ø> (-0.12%) ⬇️
unittests 85.65% <ø> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leewujung
Copy link
Copy Markdown
Member Author

Looks like this solves the reported problem! I'll go ahead to merge this now. Thanks @ctuguinay for checking this. Onward to fix the next warning!

@leewujung leewujung merged commit b34a810 into OSOceanAcoustics:main Apr 19, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in Echopype 2026 Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Anything related to dependencies

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants