Fix attribute array comparison
🚀 Pull Request
Description
Fixes: #6027
Logic for comparing arrays in attributes was relying on old numpy behaviour where an array compared to another any object would return a scalar True or False. Current numpy behaviour is to do a element-wise comparison which throws an error if the arrays are of different size.
Fix is to use the numpy.array_equal.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 89.83%. Comparing base (
8c213f0) to head (50b624d). Report is 69 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #6181 +/- ##
==========================================
- Coverage 89.83% 89.83% -0.01%
==========================================
Files 88 88
Lines 23211 23208 -3
Branches 4316 4316
==========================================
- Hits 20852 20849 -3
Misses 1627 1627
Partials 732 732
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Note, this change fixes what I believe is a bug here: https://github.com/SciTools/iris/blob/d3071ff90f9e347ad4f1e1b9b097a74f28beb21d/lib/iris/_merge.py#L388-L391 Line 391 should be checking for any differences in the array elements, not all.
It also cleans up a now unnecessary try...except block that I believe was only needed for older versions of numpy.
https://github.com/SciTools/iris/blob/d3071ff90f9e347ad4f1e1b9b097a74f28beb21d/lib/iris/common/mixin.py#L108-L111
I have extended the existing unit test to check for correct handling inequality.
However, I question the validity of this test
https://github.com/SciTools/iris/blob/d3071ff90f9e347ad4f1e1b9b097a74f28beb21d/lib/iris/tests/unit/common/mixin/test_LimitedAttributeDict.py#L45-L49
which now fails as it is testing a scalar value against a single element array. With the old logic, this would work, but it is flawed as np.all( np.array([0]) == 0 ) and np.all( np.zeros(100) == 0 ) would both return True, even though the latter is not what is intended.
I have commented out this test at the moment and would appreciate the opinion of @pp-mo.