fix: restore fill_value=None for zarr_format=2#3198
Merged
d-v-b merged 11 commits intozarr-developers:mainfrom Jul 3, 2025
Merged
fix: restore fill_value=None for zarr_format=2#3198d-v-b merged 11 commits intozarr-developers:mainfrom
d-v-b merged 11 commits intozarr-developers:mainfrom
Conversation
Introduce a new Singleton sentinel class to indicate that the default scalar for the dtype should be used as the fill_value. This allows for preserving the zarr_format 2 behavior of None -> null. For zarr format 3 either the DefaulFillValue or None imply using the dtype default scalar value.
d-v-b
reviewed
Jul 2, 2025
d-v-b
reviewed
Jul 3, 2025
d-v-b
reviewed
Jul 3, 2025
d-v-b
reviewed
Jul 3, 2025
d-v-b
reviewed
Jul 3, 2025
| dtype: ZDTypeLike | None = None, | ||
| compressor: CompressorLike = "auto", | ||
| fill_value: Any | None = None, # TODO: need type | ||
| fill_value: Any | None = DEFAULT_FILL_VALUE, # TODO: need type |
Contributor
There was a problem hiding this comment.
for posterity, we now have a type alias for the scalar types of all the dtypes which we can use instead of Any here
Contributor
There was a problem hiding this comment.
although maybe we should just use object, since we do want to allow inputs to be castable
d-v-b
approved these changes
Jul 3, 2025
Contributor
d-v-b
left a comment
There was a problem hiding this comment.
thanks for this ian! In the interest of velocity I pushed some changes. I'm going to merge this as soon as all the checks pass.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3198 +/- ##
=======================================
Coverage 94.48% 94.48%
=======================================
Files 78 78
Lines 8606 8609 +3
=======================================
+ Hits 8131 8134 +3
Misses 475 475
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Should fix #3197
Without this it is not possible to write "null" to the zarr format 2 fill value.
@d-v-b
As discussed in #3197 adds a new sentinel class to indicate using the default fill value for the dtype. Which gives:
nullnullfor a fill value input of
NoneTest comign once I get them to run locally (figuring out hatch)
TODO:
Add unit tests and/or doctests in docstrings
[NA] New/modified features documented in
docs/user-guide/*.rst[NA] Changes documented as a new file in
changes/GitHub Actions have all passed
Test coverage is 100% (Codecov passes)