Skip to content

fix: raise a validation error for out-of-bounds integer fill values - #4492

Open
barlowa124 wants to merge 9 commits into
zarr-developers:mainfrom
barlowa124:int-fill-range-4453
Open

barlowa124 wants to merge 9 commits into
zarr-developers:mainfrom
barlowa124:int-fill-range-4453

Conversation

@barlowa124

Copy link
Copy Markdown
Contributor

Summary

Partial fix for #4453, item 4 first bullet. ArrayV3Metadata.from_dict
with an out-of-range integer fill value leaked NumPy's OverflowError
instead of raising a validation error:

ArrayV3Metadata.from_dict({**ARRAY, "data_type": "int8", "fill_value": 300})
# before: OverflowError: Python integer 300 out of bounds for int8
# after:  TypeError: Invalid type: 300. Integer is out of bounds for int8.

BaseInt.from_json_scalar now bounds-checks the parsed integer against
np.iinfo before casting, so all three accepted JSON forms (int,
integer-valued float, integer string) get the same TypeError the
non-integer path already raises. In-range behavior is unchanged.

For reviewers

The parse path only: cast_scalar still uses NumPy cast semantics, so an
out-of-range fill passed through the create API keeps its current
behavior. The remaining item-4 bullets (byte-array bytes fills, r*
raw-bits dtypes) are not addressed here.

Author attestation

  • 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
  • New/modified features documented in docs/user-guide/*.md
  • Changes documented as a new file in changes/
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

barlowa124 and others added 2 commits October 8, 2026 15:30
from_json_scalar cast the parsed integer into the native dtype without a
range check, so fill_value 300 for int8 surfaced a numpy OverflowError
instead of a validation error. Bounds-check against np.iinfo before
casting; int, intish float, and intish string forms are covered.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.71%. Comparing base (19e84dc) to head (1c3bb48).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4492      +/-   ##
==========================================
+ Coverage   94.69%   94.71%   +0.01%     
==========================================
  Files          94       94              
  Lines       13619    13666      +47     
==========================================
+ Hits        12897    12944      +47     
  Misses        722      722              
Files with missing lines Coverage Δ
src/zarr/core/dtype/npy/int.py 99.38% <100.00%> (+<0.01%) ⬆️

... and 3 files with indirect coverage changes

🚀 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.

Comment thread src/zarr/core/dtype/npy/int.py Outdated

info = np.iinfo(self.to_native_dtype())
if value < info.min or value > info.max:
raise TypeError(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use ValueError. An out-of-bounds int is still an int, so it's not a TypeError

@d-v-b

d-v-b commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

The parse path only: cast_scalar still uses NumPy cast semantics, so an
out-of-range fill passed through the create API keeps its current
behavior. The remaining item-4 bullets (byte-array bytes fills, r*
raw-bits dtypes) are not addressed here.

we should use the same parsing path for both, so users see consistent errors

@d-v-b d-v-b left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requested changes:

  1. Raise ValueError for an out-of-range value, not TypeError, and drop "Invalid type" from the message. Use {data!r} so a string fill like "300" shows as a string.
  2. Update the Raises section of from_json_scalar's docstring to cover the out-of-range case.
  3. Split test_out_of_bounds_integer_from_json_scalar into two parametrized tests, one for valid values and one for out-of-range values, so one failing case doesn't hide the rest.
  4. Use single backticks in changes/4492.bugfix.md, like the other fragments, and say the fix covers reading stored metadata only.

Optional: share the bounds check with cast_scalar, so create_array(dtype="i1", fill_value=300) raises the same error instead of numpy's OverflowError.

d-v-b and others added 4 commits October 9, 2026 12:58
…shared bounds check

- Raise ValueError (not TypeError) for out-of-range values and drop
  "Invalid type" from the message; format input with {data!r}
- Document the ValueError in the Raises sections of from_json_scalar
  and cast_scalar
- Split the regression test into parametrized in-bounds and
  out-of-bounds tests
- Share the bounds check with cast_scalar so create_array with an
  out-of-range fill_value raises the same error (optional item)
- Changelog: single backticks, cover both read and write paths
@barlowa124

Copy link
Copy Markdown
Contributor Author

e360200 covers the review: out-of-range ints raise ValueError carrying {data!r}, Raises sections are updated, the bounds test is split into parametrized valid and invalid halves, plus the changelog uses single backticks. The optional item is in too: cast_scalar now calls _check_int_bounds, so an out-of-range fill_value passed through create_array gets the same ValueError as the parse path. My earlier comment described the pre-push behavior.

9ea1426 fixes a mypy arg-type error in the new test.

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.

2 participants