Skip to content

Fix for empty consolidated metadata dataset: backwards compatibility w/ 2.x #2695

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

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

mannreis
Copy link
Contributor

@mannreis mannreis commented Jan 13, 2025

This is a quick fix for the problem reported in #2694 where an empty consolidated dataset created with zarr-python 2.x fails to being read by zarr-python 3.x because of the absence of .zattrs from the store.

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@d-v-b
Copy link
Contributor

d-v-b commented Jan 13, 2025

we should add a case in our consolidated metadata tests for this, happy to provide guidance there if needed

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.29%. Comparing base (eaf5d7a) to head (3fab702).
Report is 426 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2695      +/-   ##
==========================================
- Coverage   99.98%   90.29%   -9.70%     
==========================================
  Files          38       59      +21     
  Lines       14718     6468    -8250     
==========================================
- Hits        14716     5840    -8876     
- Misses          2      628     +626     
Files with missing lines Coverage Δ
src/zarr/core/group.py 92.62% <100.00%> (ø)

... and 96 files with indirect coverage changes

@mannreis
Copy link
Contributor Author

mannreis commented Jan 13, 2025

we should add a case in our consolidated metadata tests for this, happy to provide guidance there if needed

I just added a simple tests that is able trigger this issue (before the fix) but I'm not so familiar with the code base to know if this is the best place and way to check for this

@dstansby dstansby added the needs release notes Automatically applied to PRs which haven't added release notes label Jan 13, 2025
@github-actions github-actions bot removed the needs release notes Automatically applied to PRs which haven't added release notes label Jan 14, 2025
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.

Thanks for the fix! I just cleaned up the release note a little bit.

@dstansby dstansby enabled auto-merge (squash) January 14, 2025 10:00
@dstansby dstansby merged commit 0220e45 into zarr-developers:main Jan 14, 2025
30 checks passed
@jhamman jhamman changed the title Ensure backwards compatibility Fix for empty consolidated metadata dataset: backwards compatibility w/ 2.x Jan 17, 2025
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.

Backwards compatibility reading "old" consolidated dataset without attributtes
3 participants