BUG: Make mask_invalid consistent with mask_where if copy is set to False

[cmarmo]

This pull requests makes mask_invalid consistent with mask_where when copy is set to False.
Fixes #19332.

I'd rather make the two functions consistent than change the documentation.
Also from the documentation

Only applies to arrays with a dtype where NaNs or infs make sense

I added a test checking that an error is thrown when isinfinite is not applicable.

Thanks for considering it.

[bsipocz]

Was there a particular reason for this try/except, or is it a debug remnant?

Suggested change

	try:
	return masked_where(~(np.isfinite(getdata(a))), a, copy=copy)
	except TypeError:
	raise
	return masked_where(~(np.isfinite(getdata(a))), a, copy=copy)

[cmarmo]

Indeed, I can't recall why I put the try/except. Suggestion applied. Thanks!

[seberg]

We were looking at this in the triage meeting as well. I guess the test is great (I honestly still need to parse it fully).
But we are missing an additional test for the successful path that was fixed I think: Checking that the array was indeed modified in-place with copy=False?

[cmarmo]

I have added a new test in test_masked_array_no_copy() ... just to explain myself: I didn't add it in the first place because mask_invalid becomes a straight call of masked_where which is already tested. But I guess the more tests we have the better?
Thanks!

[seberg]

Yeah, thanks. It is true that masked_where is tested, but it is also just nice to see the fix in action in the PR and we mainly have integration tests anyway.

A test too much cannot hurt :).

I find it odd that we consider inf invalid by default, but that is not a change.

Thanks @cmarmo, lets get this in!

[seberg]

I suppose we should do this just to align things anyway...

Although, now I actually suspect that this may have been intentional (Chesterton's fence greets): The function always replaces the mask, but does not always copy the data, which is a pattern to the madness that does make sense, the docs are probably just fuzzy on that intention (if it was the intention).
Sorry, long shot, but @ahaldane do you happen to have a gut feeling on this?

[InessaPawson]

@mhvk Do you have any thoughts on this?

[mhvk]

Not really. Overall, to me this PR makes sense: do as the doc states and just call masked_where. I've never understood why if data is kept, masks are not, though clearly it was designed that way.

[seberg]

Thanks @cmarmo and Marten, lets give this a shot then!

[charris]

Should probably add a release note for this.

[seberg]

Hmmm, maybe better. @cmarmo are you interested in having a look at that, the instructions are in numpy/doc/release/upcoming_changes/README.txt (you basically add a file in that folder with a 22046.change.rst as name).

Otherwise, hopefully I will remember to follow up and do it :).

[bsipocz]

Hmm, if the label is used consistently, a script could help to look for missing release notes before release time?

[cmarmo]

@cmarmo are you interested in having a look at that, the instructions are in numpy/doc/release/upcoming_changes/README.txt (you basically add a file in that folder with a 22046.change.rst as name).

I can take care of that if it can wait until the week-end. :)