Skip to content

BUG: ma.__array_wrap__ no longer propagates a dtype-invalid fill_value - #32438

Closed
CAOShurong wants to merge 1 commit into
numpy:mainfrom
CAOShurong:codex/32401-stale-fill-value
Closed

CAOShurong wants to merge 1 commit into
numpy:mainfrom
CAOShurong:codex/32401-stale-fill-value

Conversation

@CAOShurong

Copy link
Copy Markdown

Description

When a ufunc changes the output dtype of a MaskedArray (e.g. np.strings.find applied to a string masked array returns int64 match positions), the result inherits the raw _fill_value from its input via _update_from inside MaskedArray.__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_value through __array_finalize__ and raises:

TypeError: Cannot convert fill_value N/A to dtype int64

Minimal reproducer (from gh-32401):

import numpy as np
col = np.ma.MaskedArray(['foo', 'bar', 'baz'], mask=False, fill_value='N/A')
result = np.strings.find(col, 'foo')   # dtype changes str -> int64
result.view(np.ma.MaskedArray)         # TypeError before this patch

This also breaks astropy's MaskedColumn, whose .data property calls .view(np.ma.MaskedArray) internally on every __getitem__ (astropy/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:

  • a fill value that is convertible to the new dtype is kept (converted), so e.g. a numeric fill value survives an str -> int transition;
  • a fill value that cannot be represented is dropped, so the output falls back to its default fill value instead of carrying an invalid one;
  • same-dtype ufuncs are untouched (the check is gated on result.dtype != self.dtype), preserving all existing behavior.

Testing

Two regression tests added to numpy/ma/tests/test_core.py::TestUfuncs:

  1. 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 reported TypeError.
  2. test_dtype_changing_ufunc_keeps_convertible_fill_value — asserts a numeric fill value on a string array survives a str -> int64 ufunc (converted, not discarded).

Full suite with the patch applied:

numpy/ma/tests/test_core.py      4172 passed, 1 skipped, 2 xfailed
numpy/ma/tests/test_extras.py    }
numpy/ma/tests/test_regression.py } 185 passed
numpy/ma/tests/test_mrecords.py  }

Notes for reviewers

  • The failure mode is "stale state discovered late", so I chose revalidation at wrap time over raising eagerly at wrap time — an eager raise would turn previously-working code paths (any dtype-changing ufunc on an array that happens to carry a fill value) into hard errors. Dropping to the default fill value keeps those paths functional while guaranteeing internal consistency.
  • An alternative considered was validating inside _update_from itself, 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.

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
@mattip

mattip commented Aug 26, 2026

Copy link
Copy Markdown
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.

@mattip mattip closed this Aug 26, 2026
@CAOShurong

Copy link
Copy Markdown
Author

Understood, and apologies for skipping the PR template — that was careless of me. I've read the AI policy and will follow it going forward.

I won't reopen this; gh-32401 is already covered by gh-32423. Thanks for keeping NumPy's contribution standards clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants