Fix array_equal behaviour for masked arrays

[stephenworsley]

Current behaviour will compare the underlying data of a masked array and will sometimes compare masked arrays as unequal when they are otherwise equal for all unmasked points. This would cause an error in an iris-esmf-regrid PR as described here:
SciTools-incubator/iris-esmf-regrid#138 (comment)

[rcomer]

Seems that ignoring the masks was deliberate, but only because “the chosen approach fixes the problem [of all-masked data raising errors], is simpler, and reflects numpy behaviour.” #905

Perhaps using something like ma.filled would help?

[bjlittle]

Related to numpy/numpy#16022 (comment), which was merged into numpy, but then reverted (as it broke scipy and astropy)

[trexfeathers]

If we're worried about upsetting the default Iris behaviour, perhaps the neatest solution would be an iris.util function that provides the alternative equality behaviour?

As far as I'm aware the only calls for this alternative behaviour are cases where the caller is explicitly checking equality (rather than wanting internal Iris behaviour to be changed).

[rcomer]

There is also a relevant thread at numpy/numpy#14624

I think what Iris does in testing is sensible:

iris/lib/iris/tests/__init__.py

Lines 590 to 668 in 56a97d6

	def _assertMaskedArray(self, assertion, a, b, strict, **kwargs):
	# Define helper function to extract unmasked values as a 1d
	# array.
	def unmasked_data_as_1d_array(array):
	array = ma.asarray(array)
	if array.ndim == 0:
	if array.mask:
	data = np.array([])
	else:
	data = np.array([array.data])
	else:
	data = array.data[~ma.getmaskarray(array)]
	return data
	# Compare masks. This will also check that the array shapes
	# match, which is not tested when comparing unmasked values if
	# strict is False.
	a_mask, b_mask = ma.getmaskarray(a), ma.getmaskarray(b)
	np.testing.assert_array_equal(a_mask, b_mask)
	if strict:
	assertion(a.data, b.data, **kwargs)
	else:
	assertion(
	unmasked_data_as_1d_array(a),
	unmasked_data_as_1d_array(b),
	**kwargs,
	)
	def assertMaskedArrayEqual(self, a, b, strict=False):
	"""
	Check that masked arrays are equal. This requires the
	unmasked values and masks to be identical.
	Args:
	* a, b (array-like):
	Two arrays to compare.
	Kwargs:
	* strict (bool):
	If True, perform a complete mask and data array equality check.
	If False (default), the data array equality considers only unmasked
	elements.
	"""
	self._assertMaskedArray(np.testing.assert_array_equal, a, b, strict)
	def assertArrayAlmostEqual(self, a, b, decimal=6):
	np.testing.assert_array_almost_equal(a, b, decimal=decimal)
	def assertMaskedArrayAlmostEqual(self, a, b, decimal=6, strict=False):
	"""
	Check that masked arrays are almost equal. This requires the
	masks to be identical, and the unmasked values to be almost
	equal.
	Args:
	* a, b (array-like):
	Two arrays to compare.
	Kwargs:
	* strict (bool):
	If True, perform a complete mask and data array equality check.
	If False (default), the data array equality considers only unmasked
	elements.
	* decimal (int):
	Equality tolerance level for
	:meth:`numpy.testing.assert_array_almost_equal`, with the meaning
	'abs(desired-actual) < 0.5 * 10**(-decimal)'
	"""
	self._assertMaskedArray(
	np.testing.assert_array_almost_equal, a, b, strict, decimal=decimal
	)

[stephenworsley]

There is also a relevant thread at numpy/numpy#14624

@rcomer It's interesting that there is an inconsistency between different numpy functions at the moment, though given that currently iris.util.array_equal behaves in the same way as np.array_equal, that could be enough of a reason to keep this function behaving the same. There's still potentially a question of how we use iris.util.array_equal within iris. It might make sense in places (such ass coord equality) to use an approach of calling ma.filled as you suggest before calling iris.util.array_equal and also checking that the mask is equal.

[github-actions]

In order to maintain a backlog of relevant PRs, we automatically label them as stale after 500 days of inactivity.

If this PR is still important to you, then please comment on this PR and the stale label will be removed.

Otherwise this PR will be automatically closed in 28 days time.

[github-actions]

This stale PR has been automatically closed due to a lack of community activity.

If you still care about this PR, then please either:

• Re-open this PR, if you have sufficient permissions, or
• Add a comment pinging @SciTools/iris-devs who will re-open on your behalf.

[pp-mo]

As discussed @trexfeathers @stephenworsley @bjlittle @pp-mo ...
This is probably still relevant
Causes problem here : https://github.com/SciTools-incubator/iris-esmf-regrid/blob/v0.9.0/esmf_regrid/schemes.py#L1377-L1396

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.78%. Comparing base (167d149) to head (9045abc).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4457   +/-   ##
=======================================
  Coverage   89.78%   89.78%           
=======================================
  Files          90       90           
  Lines       22938    22945    +7     
  Branches     5023     5026    +3     
=======================================
+ Hits        20595    20602    +7     
  Misses       1612     1612           
  Partials      731      731           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pp-mo]

Pretty much OK I think -- just a couple of comments.
New tests are good.
I'm a bit surprised this hasn't triggered test failures elsewhere, but I guess it's a bit niche.

[pp-mo]

All done. apart from the extra point I just added.

[pp-mo]

Sorry I agreed with your latest, but I shifted my ground so I still think there is something to fix here..
Not long now, I should think !

[pp-mo]

Great, thanks for your patience @stephenworsley !