diff --git a/.github/workflows/benchmarks_run.yml b/.github/workflows/benchmarks_run.yml index f32d42caaf..287d84123f 100644 --- a/.github/workflows/benchmarks_run.yml +++ b/.github/workflows/benchmarks_run.yml @@ -78,7 +78,7 @@ jobs: - name: Install Nox run: | - pip install nox + pip install nox!=2025.05.01 - name: Cache environment directories id: cache-env-dir diff --git a/docs/src/whatsnew/3.12.rst b/docs/src/whatsnew/3.12.rst index 2b6ca8ab17..b2e3ba5baa 100644 --- a/docs/src/whatsnew/3.12.rst +++ b/docs/src/whatsnew/3.12.rst @@ -215,6 +215,12 @@ v3.12.2 (09 May 2025) #. `@trexfeathers`_ refactored Iris loading and saving to make it compatible with Dask version ``2025.4.0`` and above. (:pull:`6451`) +#. `@trexfeathers`_ and `@ukmo-ccbunney`_ adapted array comparison in response + to NumPy v1.25 deprecating comparison of un-broadcastable arrays. It is + hoped that users will see no difference in behaviour, but please get in touch + if you notice anything. See `NumPy v1.25 expired deprecations`_ and + `numpy#22707`_ for more. (:pull:`6665`) + 📚 Documentation ================ @@ -271,3 +277,5 @@ v3.12.2 (09 May 2025) .. _SPEC 0: https://scientific-python.org/specs/spec-0000/ .. _Running setuptools commands: https://setuptools.pypa.io/en/latest/deprecated/commands.html +.. _NumPy v1.25 expired deprecations: https://numpy.org/doc/stable/release/1.25.0-notes.html#expired-deprecations +.. _numpy#22707: https://github.com/numpy/numpy/pull/22707 diff --git a/lib/iris/_concatenate.py b/lib/iris/_concatenate.py index 6caee79c4f..186ad7700b 100644 --- a/lib/iris/_concatenate.py +++ b/lib/iris/_concatenate.py @@ -16,6 +16,7 @@ from xxhash import xxh3_64 from iris._lazy_data import concatenate as concatenate_arrays +from iris.common.metadata import hexdigest import iris.coords from iris.coords import AncillaryVariable, AuxCoord, CellMeasure, DimCoord import iris.cube @@ -786,7 +787,7 @@ def _coordinate_differences(self, other, attr, reason="metadata"): diff_names = [] for self_key, self_value in self_dict.items(): other_value = other_dict[self_key] - if self_value != other_value: + if hexdigest(self_value) != hexdigest(other_value): diff_names.append(self_key) result = ( " " + reason, diff --git a/lib/iris/_constraints.py b/lib/iris/_constraints.py index 765a975651..c0ba2f120f 100644 --- a/lib/iris/_constraints.py +++ b/lib/iris/_constraints.py @@ -531,6 +531,8 @@ def __init__(self, **attributes): super().__init__(cube_func=self._cube_func) def __eq__(self, other): + # Note: equality means that NumPy arrays are not supported for + # AttributeConstraints (get the truth ambiguity error). eq = ( isinstance(other, AttributeConstraint) and self._attributes == other._attributes @@ -553,6 +555,8 @@ def _cube_func(self, cube): match = False break else: + # Note: equality means that NumPy arrays are not supported + # for AttributeConstraints (get the truth ambiguity error). if cube_attr != value: match = False break diff --git a/lib/iris/common/mixin.py b/lib/iris/common/mixin.py index dae10abc9b..bbb806f331 100644 --- a/lib/iris/common/mixin.py +++ b/lib/iris/common/mixin.py @@ -115,6 +115,9 @@ def __eq__(self, other): match = set(self.keys()) == set(other.keys()) if match: for key, value in self.items(): + # TODO: should this use the iris.common.metadata approach of + # using hexdigest? Might be a breaking change for some corner + # cases, so would need a major release. match = np.array_equal( np.array(value, ndmin=1), np.array(other[key], ndmin=1) ) diff --git a/lib/iris/coords.py b/lib/iris/coords.py index ca73dcb729..7c5cbdfdea 100644 --- a/lib/iris/coords.py +++ b/lib/iris/coords.py @@ -760,7 +760,9 @@ def is_compatible(self, other, ignore=None): ignore = (ignore,) common_keys = common_keys.difference(ignore) for key in common_keys: - if np.any(self.attributes[key] != other.attributes[key]): + if not iris.util._attribute_equal( + self.attributes[key], other.attributes[key] + ): compatible = False break diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 77191c3a9a..51a6eea5c6 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -1435,7 +1435,9 @@ def is_compatible( ignore = (ignore,) common_keys = common_keys.difference(ignore) for key in common_keys: - if np.any(self.attributes[key] != other.attributes[key]): + if not iris.util._attribute_equal( + self.attributes[key], other.attributes[key] + ): compatible = False break diff --git a/lib/iris/fileformats/_structured_array_identification.py b/lib/iris/fileformats/_structured_array_identification.py index 0f386e9815..a9a95ea516 100644 --- a/lib/iris/fileformats/_structured_array_identification.py +++ b/lib/iris/fileformats/_structured_array_identification.py @@ -111,7 +111,9 @@ def __eq__(self, other): result = NotImplemented if stride is not None or arr is not None: - result = stride == self.stride and np.all(self.unique_ordered_values == arr) + result = stride == self.stride and np.array_equal( + self.unique_ordered_values, arr + ) return result def __ne__(self, other): @@ -284,7 +286,7 @@ def from_array(cls, arr): # Do one last sanity check - does the array we've just described # actually compute the correct array? constructed_array = structure.construct_array(arr.size) - if not np.all(constructed_array == arr): + if not np.array_equal(constructed_array, arr): structure = None return structure diff --git a/lib/iris/fileformats/netcdf/saver.py b/lib/iris/fileformats/netcdf/saver.py index 4a2474ba9b..ef8c844764 100644 --- a/lib/iris/fileformats/netcdf/saver.py +++ b/lib/iris/fileformats/netcdf/saver.py @@ -2589,13 +2589,7 @@ def save( # Fnd any global attributes which are not the same on *all* cubes. def attr_values_equal(val1, val2): # An equality test which also works when some values are numpy arrays (!) - # As done in :meth:`iris.common.mixin.LimitedAttributeDict.__eq__`. - match = val1 == val2 - try: - match = bool(match) - except ValueError: - match = match.all() - return match + return iris.util._attribute_equal(val1, val2) cube0 = cubes[0] invalid_globals = set( @@ -2682,7 +2676,9 @@ def attr_values_equal(val1, val2): common_keys.intersection_update(keys) different_value_keys = [] for key in common_keys: - if np.any(attributes[key] != cube.attributes[key]): + if not iris.util._attribute_equal( + attributes[key], cube.attributes[key] + ): different_value_keys.append(key) common_keys.difference_update(different_value_keys) local_keys.update(different_value_keys) diff --git a/lib/iris/fileformats/pp_load_rules.py b/lib/iris/fileformats/pp_load_rules.py index 71540fe74a..59e0f31d17 100644 --- a/lib/iris/fileformats/pp_load_rules.py +++ b/lib/iris/fileformats/pp_load_rules.py @@ -139,8 +139,15 @@ def _convert_vertical_coords( ) coords_and_dims.append((coord, dim)) + # Common calc for Depth + try: + svd_lev_eq = brsvd1 == brlev + except ValueError: + # In case of broadcasting errors. + svd_lev_eq = False + # Depth - unbound. - if (len(lbcode) != 5) and (lbvc == 2) and np.all(brsvd1 == brlev): + if (len(lbcode) != 5) and (lbvc == 2) and np.all(svd_lev_eq): coord = _dim_or_aux( blev, standard_name="depth", @@ -150,7 +157,7 @@ def _convert_vertical_coords( coords_and_dims.append((coord, dim)) # Depth - bound. - if (len(lbcode) != 5) and (lbvc == 2) and np.all(brsvd1 != brlev): + if (len(lbcode) != 5) and (lbvc == 2) and np.all(~svd_lev_eq): coord = _dim_or_aux( blev, standard_name="depth", @@ -164,10 +171,10 @@ def _convert_vertical_coords( if ( (len(lbcode) != 5) and (lbvc == 2) - and (np.any(brsvd1 == brlev) and np.any(brsvd1 != brlev)) + and (np.any(svd_lev_eq) and np.any(~svd_lev_eq)) ): - lower = np.where(brsvd1 == brlev, blev, brsvd1) - upper = np.where(brsvd1 == brlev, blev, brlev) + lower = np.where(svd_lev_eq, blev, brsvd1) + upper = np.where(svd_lev_eq, blev, brlev) coord = _dim_or_aux( blev, standard_name="depth", @@ -189,7 +196,7 @@ def _convert_vertical_coords( units="1", ) coords_and_dims.append((coord, dim)) - elif np.any(brsvd1 != brlev): + elif np.any(~svd_lev_eq): # UM populates metadata CORRECTLY, # so treat it as the expected (bounded) soil depth. coord = _dim_or_aux( diff --git a/lib/iris/tests/test_coding_standards.py b/lib/iris/tests/test_coding_standards.py index 3c7e524e1e..1e71aad3b3 100644 --- a/lib/iris/tests/test_coding_standards.py +++ b/lib/iris/tests/test_coding_standards.py @@ -223,8 +223,10 @@ def last_change_by_fname(): # Call "git whatchanged" to get the details of all the files and when # they were last changed. + # TODO: whatchanged is deprecated, find an alternative Git command. output = subprocess.check_output( - ["git", "whatchanged", "--pretty=TIME:%ct"], cwd=IRIS_REPO_DIRPATH + ["git", "whatchanged", "--pretty=TIME:%ct", "--i-still-use-this"], + cwd=IRIS_REPO_DIRPATH, ) output = output.decode().split("\n") diff --git a/lib/iris/tests/unit/coords/test_Coord.py b/lib/iris/tests/unit/coords/test_Coord.py index 97429f58f8..f81b1cc706 100644 --- a/lib/iris/tests/unit/coords/test_Coord.py +++ b/lib/iris/tests/unit/coords/test_Coord.py @@ -831,6 +831,12 @@ def test_different_array_attrs_incompatible(self): self.other_coord.attributes["array_test"] = np.array([1.0, 2, 777.7]) self.assertFalse(self.test_coord.is_compatible(self.other_coord)) + def test_misshaped_array_attrs_incompatible(self): + # Comparison should avoid broadcast failures and return False. + self.test_coord.attributes["array_test"] = np.array([1.0, 2, 3]) + self.other_coord.attributes["array_test"] = np.array([1.0, 2]) + self.assertFalse(self.test_coord.is_compatible(self.other_coord)) + class Test_contiguous_bounds(tests.IrisTest): def test_1d_coord_no_bounds_warning(self): diff --git a/lib/iris/tests/unit/cube/test_Cube.py b/lib/iris/tests/unit/cube/test_Cube.py index 21fa27d2f9..2a52478c45 100644 --- a/lib/iris/tests/unit/cube/test_Cube.py +++ b/lib/iris/tests/unit/cube/test_Cube.py @@ -876,6 +876,12 @@ def test_different_array_attrs_incompatible(self): self.other_cube.attributes["array_test"] = np.array([1.0, 2, 777.7]) assert not self.test_cube.is_compatible(self.other_cube) + def test_misshaped_array_attrs_incompatible(self): + # Comparison should avoid broadcast failures and return False. + self.test_cube.attributes["array_test"] = np.array([1.0, 2, 3]) + self.other_cube.attributes["array_test"] = np.array([1.0, 2]) + assert not self.test_cube.is_compatible(self.other_cube) + class Test_rolling_window: @pytest.fixture(autouse=True) diff --git a/lib/iris/tests/unit/fileformats/netcdf/saver/test_save.py b/lib/iris/tests/unit/fileformats/netcdf/saver/test_save.py index 860da84e6b..b4b06c8c33 100644 --- a/lib/iris/tests/unit/fileformats/netcdf/saver/test_save.py +++ b/lib/iris/tests/unit/fileformats/netcdf/saver/test_save.py @@ -79,6 +79,21 @@ def test_attributes_arrays(self): ds.close() self.assertArrayEqual(res, np.arange(2)) + def test_attributes_arrays_incompatible_shapes(self): + # Ensure successful comparison without raising a broadcast error. + c1 = Cube([1], attributes={"bar": np.arange(2)}) + c2 = Cube([2], attributes={"bar": np.arange(3)}) + + with self.temp_filename("foo.nc") as nc_out: + save([c1, c2], nc_out) + ds = _thread_safe_nc.DatasetWrapper(nc_out) + with pytest.raises(AttributeError): + _ = ds.getncattr("bar") + for var in ds.variables.values(): + res = var.getncattr("bar") + self.assertIsInstance(res, np.ndarray) + ds.close() + def test_no_special_attribute_clash(self): # Ensure that saving multiple cubes with netCDF4 protected attributes # works as expected. diff --git a/lib/iris/tests/unit/fileformats/pp_load_rules/test__convert_vertical_coords.py b/lib/iris/tests/unit/fileformats/pp_load_rules/test__convert_vertical_coords.py index 7f6270e9f3..a5c239a37d 100644 --- a/lib/iris/tests/unit/fileformats/pp_load_rules/test__convert_vertical_coords.py +++ b/lib/iris/tests/unit/fileformats/pp_load_rules/test__convert_vertical_coords.py @@ -8,6 +8,7 @@ """ import numpy as np +import pytest from iris.aux_factory import HybridHeightFactory, HybridPressureFactory from iris.coords import AuxCoord, DimCoord @@ -280,6 +281,29 @@ def test_unbounded__vector_no_depth(self): dim=0, ) + def test_unbounded_incompatible_vectors(self): + # Confirm this is not vulnerable to the non-broadcastable error. + lblev = [1, 2, 3] + blev = [10, 20, 30] + brsvd1 = [5, 15, 25, 35] + brlev = [5, 15, 25] + avoided_error = "operands could not be broadcast together" + try: + self._check_depth( + _lbcode(1), + lblev=lblev, + blev=blev, + brsvd1=brsvd1, + brlev=brlev, + expect_bounds=False, + dim=1, + ) + except ValueError as err: + if avoided_error in str(err): + message = f'Test failed to avoid specified error: "{err}"' + pytest.fail(message) + pass + def test_bounded(self): self._check_depth( _lbcode(1), lblev=23.0, brlev=22.5, brsvd1=23.5, expect_bounds=True @@ -300,6 +324,29 @@ def test_bounded__vector(self): dim=1, ) + def test_bounded_incompatible_vectors(self): + # Confirm this is not vulnerable to the non-broadcastable error. + lblev = [1, 2, 3] + blev = [10, 20, 30] + brsvd1 = [5, 15, 25, 35] + brlev = [15, 25, 35] + avoided_error = "operands could not be broadcast together" + try: + self._check_depth( + _lbcode(1), + lblev=lblev, + blev=blev, + brsvd1=brsvd1, + brlev=brlev, + expect_bounds=True, + dim=1, + ) + except ValueError as err: + if avoided_error in str(err): + message = f'Test failed to avoid specified error: "{err}"' + pytest.fail(message) + pass + def test_cross_section(self): self._check_depth(_lbcode(ix=1, iy=2), lblev=23.0, expect_match=False) @@ -360,6 +407,37 @@ def test_normal__vector(self): lblev = np.arange(10) self._check_soil_level(_lbcode(0), lblev=lblev, dim=0) + def test_normal_incompatible_vectors(self): + # Confirm this is not vulnerable to the non-broadcastable error. + lbvc = 6 + stash = STASH(1, 1, 1) + lbcode = _lbcode(0) + lblev = np.arange(10) + brsvd1 = [1] * len(lblev) + brlev = brsvd1 + [1] + blev, bhlev, bhrlev, brsvd2 = None, None, None, None + + avoided_error = "operands could not be broadcast together" + try: + _ = _convert_vertical_coords( + lbcode=lbcode, + lbvc=lbvc, + blev=blev, + lblev=lblev, + stash=stash, + bhlev=bhlev, + bhrlev=bhrlev, + brsvd1=brsvd1, + brsvd2=brsvd2, + brlev=brlev, + dim=0, + ) + except ValueError as err: + if avoided_error in str(err): + message = f'Test failed to avoid specified error: "{err}"' + pytest.fail(message) + pass + def test_cross_section(self): self._check_soil_level(_lbcode(ix=1, iy=2), expect_match=False) diff --git a/lib/iris/tests/unit/fileformats/structured_array_identification/test_ArrayStructure.py b/lib/iris/tests/unit/fileformats/structured_array_identification/test_ArrayStructure.py index bf23b39a5c..bc461f84ee 100644 --- a/lib/iris/tests/unit/fileformats/structured_array_identification/test_ArrayStructure.py +++ b/lib/iris/tests/unit/fileformats/structured_array_identification/test_ArrayStructure.py @@ -93,6 +93,13 @@ def test_irregular_3d(self): a[0, 0, 0] = 5 assert self.struct_from_arr(a) is None + def test_irregular_1d(self): + # Note this tests a unique scenario where NumPy raises a broadcasting + # error (having previously allowed elementwise comparison in earlier + # versions). + a = np.array([0, 0, 0, 0, 1, 1, 1, 1, 2]) + assert self.struct_from_arr(a) is None + def test_repeated_3d(self): sub = np.array([-1, 3, 1, 2]) a = construct_nd(sub, 2, (3, 2, 4)) @@ -121,6 +128,11 @@ def test_multi_dim_array(self): with pytest.raises(ValueError): ArrayStructure.from_array(np.arange(12).reshape(3, 4)) + def test_eq_incompatible_shapes(self): + struct1 = ArrayStructure(1, np.array([1, 2])) + struct2 = ArrayStructure(1, np.array([1, 2, 3])) + assert struct1 != struct2 + class TestNdarrayAndDimsCases: """Defines the test functionality for nd_array_and_dims. This class diff --git a/lib/iris/tests/unit/util/test_describe_diff.py b/lib/iris/tests/unit/util/test_describe_diff.py index 0263a0de27..f6593d04f2 100644 --- a/lib/iris/tests/unit/util/test_describe_diff.py +++ b/lib/iris/tests/unit/util/test_describe_diff.py @@ -58,3 +58,10 @@ def test_different_array_attributes(self): "incompatible_array_attrs.str.txt", ], ) + + def test_incompatible_array_attributes(self): + # test incompatible array attribute + self.cube_a.attributes["test_array"] = np.array([1, 2, 3]) + self.cube_b.attributes["test_array"] = np.array([1, 2]) + with pytest.raises(ValueError, match="Error comparing test_array attributes"): + describe_diff(self.cube_a, self.cube_b) diff --git a/lib/iris/tests/unit/util/test_equalise_attributes.py b/lib/iris/tests/unit/util/test_equalise_attributes.py index 1392f9cff8..6aa33558d7 100644 --- a/lib/iris/tests/unit/util/test_equalise_attributes.py +++ b/lib/iris/tests/unit/util/test_equalise_attributes.py @@ -129,6 +129,18 @@ def test_array_same(self): cubes = [self.cube_a1b5v1, self.cube_a1b6v1] self._test(cubes, {"a": 1, "v": self.v1}, [{"b": 5}, {"b": 6}]) + @pytest.fixture + def make_a1b6v2_incompatible(self): + v_array = self.cube_a1b6v2.attributes["v"] + self.cube_a1b6v2.attributes["v"] = np.repeat(v_array, 2) + + def test_array_incompatible(self, make_a1b6v2_incompatible): + cubes = [self.cube_a1b5v1, self.cube_a1b6v2] + with pytest.raises( + ValueError, match=r"Error comparing \('local', 'v'\) attributes" + ): + self._test(cubes, {}, []) + @_shared_utils.skip_data def test_complex_nonecommon(self): # Example with cell methods and factories, but no common attributes. diff --git a/lib/iris/util.py b/lib/iris/util.py index e0d2fa5183..1070cb088f 100644 --- a/lib/iris/util.py +++ b/lib/iris/util.py @@ -15,7 +15,7 @@ import os.path import sys import tempfile -from typing import TYPE_CHECKING, List, Literal +from typing import TYPE_CHECKING, Any, List, Literal from warnings import warn import cf_units @@ -232,7 +232,12 @@ def describe_diff(cube_a, cube_b, output_file=None): else: common_keys = set(cube_a.attributes).intersection(cube_b.attributes) for key in common_keys: - if np.any(cube_a.attributes[key] != cube_b.attributes[key]): + try: + eq = np.all(cube_a.attributes[key] == cube_b.attributes[key]) + except ValueError as err: + message = f"Error comparing {key} attributes: {err}" + raise ValueError(message) + if not eq: output_file.write( '"%s" cube_a attribute value "%s" is not ' "compatible with cube_b " @@ -390,6 +395,24 @@ def _rolling_window(array): return rw +def _attribute_equal( + attr1: Any, + attr2: Any, +) -> bool: + """Compare two attribute values, including np arrays. + + If either attribute is a NumPy array, :func:`numpy.array_equal` is used, to + avoid broadcastability errors in case of mismatches. + """ + # TODO: at next major release replace uses of this with hexdigest + # comparisons, in alignment with iris.common.metadata (consider calling + # a routine in iris.common.metadata). + if isinstance(attr1, np.ndarray) or isinstance(attr2, np.ndarray): + return np.array_equal(attr1, attr2) + else: + return attr1 == attr2 + + def _masked_array_equal( array1: np.ndarray, array2: np.ndarray, @@ -427,7 +450,12 @@ def _masked_array_equal( else: ignore |= nanmask - eqs = ma.getdata(array1) == ma.getdata(array2) + try: + eqs = ma.getdata(array1) == ma.getdata(array2) + except ValueError: + # In case of broadcasting errors. + eqs = np.zeros(array1.shape, dtype=bool) + if ignore is not None: eqs = np.where(ignore, True, eqs) @@ -2106,11 +2134,15 @@ def equalise_attributes(cubes): for attrs in cube_attrs[1:]: cube_keys = list(attrs.keys()) keys_to_remove.update(cube_keys) - common_keys = [ - key - for key in common_keys - if (key in cube_keys and np.all(attrs[key] == cube_attrs[0][key])) - ] + + def eq(key): + try: + return np.all(attrs[key] == cube_attrs[0][key]) + except ValueError as err: + message = f"Error comparing {key} attributes: {err}" + raise ValueError(message) + + common_keys = [key for key in common_keys if (key in cube_keys and eq(key))] keys_to_remove.difference_update(common_keys) # Convert back from the resulting 'paired' keys set, extracting just the