BUG: Reset an unrepresentable fill_value on a dtype change (#32508) - #32557
Merged
Merged
Conversation
Co-authored-by: Yeonho Kim <[email protected]>
The backport of numpy#32508 brought with it a number of new tests, this fixes a test failure by adding a needed import.
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.
Backport of #32508 and #32358.
PR summary
As discussed in gh-28255. Since #32423,
MaskedArray._update_fromvalidates an inheritedfill_valueagainst the new dtype and falls back to the default when the cast fails. It decides "fails" by catching an exception, which misses casts that fail through the floating point error state instead.1e20is not a value the user picked, it is the default fill_value for float64, and reading the attribute stores it as a 0-d array. Casting that storednp.float64(1e20)to int64 does not raise: it sets theinvalidflag, warns, and returns an out-of-range integer (INT64_MIN on x86; INT64_MAX on the macOS machine in the issue thread). A Python float1e20passed directly goes through the scalar path and raises, which is whyfill_value=1e20at construction already fails cleanly.This wraps that call in
np.errstate(invalid='raise')and addsFloatingPointErrorto the except clause, so the fallback added in #32423 also runs for this case.Scope:
invalidis raised.overis left alone on purpose: a float fill_value that saturates toinfin a narrower float dtype is still a usable fill_value, and replacing it with the default would not be an improvement.invalidflag is platform dependent.-1.0touint8sets no flag on x86 and wraps to 255, so this change does not touch it there; on platforms where it does set the flag (BUG: MaskedArray.astype('uint8') with certain fill_value raises warning on ARM (Mac M3) inside Docker (Ubuntu 24.04/25.04) and leads inconsistent output #28403), the fill_value now falls back to the default instead of a platform-specific value.arr.fill_value = ...still raises, since the setter calls_check_fill_valuedirectly.fill_valuestoring the default, separating default and user fill values) are not addressed here.Tests: the new test compares against the result when
fill_valuewas never read rather than against a specific number, so it does not depend on the platform or on the default integer width. In gh-28255 I said this would not touch existing tests; running the fullnumpy/masuite showed thatTestMaskedArrayFunctions.test_whereinnumpy/ma/tests/test_core.pyasserted theRuntimeWarningfrom exactly this cast (set_fill_value(1e20)followed byastype(int)). It now asserts that the cast emits no warning and thatixm.fill_valueis the default for the new dtype. The rest ofnumpy/mapasses unchanged (run against 2.5.2 with the change applied).Fixes #28255
First time committer introduction
Not my first PR (#32405 was a whitespace fix), but my first change to
numpy.ma. I came tonumpy.mathrough gh-15601, which I am also preparing a fix for, and found this one while going through openmaissues around the time #32423 was being reviewed.AI Disclosure
I used Claude throughout: to help investigate the cause, to draft the two-line change, the tests, and the release note from my notes, and to put this description and the code comments into English. I reviewed and applied the changes, and ran the reproducer, the new tests, and the
numpy/masuite myself.