Skip to content

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Jul 2, 2025

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:

Version Format 2 Format 3
3.0.9 null default scalar
This PR null default scalar
Main default scalar default scalar

for a fill value input of None

Test 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)

@github-actions github-actions bot added the needs release notes Automatically applied to PRs which haven't added release notes label Jul 2, 2025
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.
@github-actions github-actions bot removed the needs release notes Automatically applied to PRs which haven't added release notes label 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
Copy link
Contributor

Choose a reason for hiding this comment

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

for posterity, we now have a type alias for the scalar types of all the dtypes which we can use instead of Any here

Copy link
Contributor

Choose a reason for hiding this comment

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

although maybe we should just use object, since we do want to allow inputs to be castable

Copy link
Contributor

@d-v-b d-v-b 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 this ian! In the interest of velocity I pushed some changes. I'm going to merge this as soon as all the checks pass.

@d-v-b d-v-b enabled auto-merge (squash) July 3, 2025 09:14
@d-v-b d-v-b added this to the 3.1.0 milestone Jul 3, 2025
@d-v-b d-v-b merged commit a7080a3 into zarr-developers:main Jul 3, 2025
48 of 50 checks passed
@ianhi ianhi deleted the debug-fill-value-issue-3197 branch July 3, 2025 15:04
Copy link

codecov bot commented Oct 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.48%. Comparing base (5a24487) to head (b3b07aa).
⚠️ Report is 128 commits behind head on main.

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           
Files with missing lines Coverage Δ
src/zarr/api/synchronous.py 94.66% <100.00%> (ø)
src/zarr/core/array.py 98.46% <100.00%> (-0.13%) ⬇️

... and 1 file with indirect coverage changes

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

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.

Breaking Change in Fill Value Behavior from 3.0.9 to main for zarr format 2

2 participants