BUG: ma.__array_wrap__ no longer propagates a dtype-invalid fill_value - #32438
Closed
CAOShurong wants to merge 1 commit into
Closed
CAOShurong wants to merge 1 commit into
CAOShurong wants to merge 1 commit into
Conversation
When a ufunc changed the output dtype of a MaskedArray (e.g.
np.strings.find mapping a string array to int64 positions),
MaskedArray.__array_wrap__ copied the input's raw _fill_value onto the
result via _update_from without validating it against the new dtype.
The inconsistent state did not surface immediately -- repr and indexing
worked -- but the next .view() re-ran _check_fill_value through
__array_finalize__ and raised TypeError ("Cannot convert fill_value
N/A to dtype int64"). This broke astropy MaskedColumn, whose .data
property calls .view(np.ma.MaskedArray) internally on every __getitem__
(astropy#20257).
Fix: after _update_from in __array_wrap__, if the result dtype differs
from the input dtype, revalidate the copied fill_value with
_check_fill_value. Values that are convertible to the new dtype are
kept (converted); values that are not are dropped so the output falls
back to its default fill_value instead of carrying an invalid one.
Regression tests cover both directions plus the original reproducer.
Fixes numpygh-32401
Member
|
You ignored the PR template, so I'm going to assume that this PR has been made with agentic AI. This violates our AI policy; we prefer to talk to humans. |
Author
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.
Description
When a ufunc changes the output dtype of a
MaskedArray(e.g.np.strings.findapplied to a string masked array returns int64 match positions), the result inherits the raw_fill_valuefrom its input via_update_frominsideMaskedArray.__array_wrap__, without any validation against the new dtype.That inconsistent state does not surface immediately — repr, indexing, etc. all work — but the next
.view()re-runs_check_fill_valuethrough__array_finalize__and raises:Minimal reproducer (from gh-32401):
This also breaks astropy's
MaskedColumn, whose.dataproperty calls.view(np.ma.MaskedArray)internally on every__getitem__(astropy/astropy#20257).Fix: after
_update_fromin__array_wrap__, if the result dtype differs from the input dtype, revalidate the copied_fill_valuewith_check_fill_value:str -> inttransition;result.dtype != self.dtype), preserving all existing behavior.Testing
Two regression tests added to
numpy/ma/tests/test_core.py::TestUfuncs:test_dtype_changing_ufunc_does_not_inherit_invalid_fill_value— the issue's reproducer; asserts the invalid fill value is not carried over and.view(MaskedArray)no longer raises. Fails on unpatched main with the reportedTypeError.test_dtype_changing_ufunc_keeps_convertible_fill_value— asserts a numeric fill value on a string array survives astr -> int64ufunc (converted, not discarded).Full suite with the patch applied:
Notes for reviewers
_update_fromitself, but that function is used by many non-ufunc callers (__array_finalize__, explicit attribute copies) where silently dropping the caller-provided fill value would be more surprising.