diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 43bacd3ec55..270046164e8 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -46,7 +46,7 @@ jobs: session: "tests" env: - IRIS_TEST_DATA_VERSION: "2.16" + IRIS_TEST_DATA_VERSION: "2.17" ENV_NAME: "ci-tests" steps: diff --git a/docs/src/whatsnew/latest.rst b/docs/src/whatsnew/latest.rst index a4204941578..51fea14b294 100644 --- a/docs/src/whatsnew/latest.rst +++ b/docs/src/whatsnew/latest.rst @@ -57,6 +57,13 @@ This document explains the changes made to Iris for this release variables and cell measures that map to a cube dimension of length 1 are now included in the respective vector sections. (:pull:`4945`) +#. `@pp-mo`_ fixed a bug in NAME loaders where data with no associated statistic would + load as a cube with invalid cell-methods, which cannot be printed or saved to netcdf. + (:issue:`3288`, :pull:`4933`) + +#. `@pp-mo`_ ensured that :data:`iris.cube.Cube.cell_methods` must always be an iterable + of :class:`iris.coords.CellMethod` objects (:pull:`4933`). + 💣 Incompatible Changes ======================= diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 9e49b26ac57..9779558506c 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -2299,10 +2299,23 @@ def cell_methods(self): return self._metadata_manager.cell_methods @cell_methods.setter - def cell_methods(self, cell_methods): - self._metadata_manager.cell_methods = ( - tuple(cell_methods) if cell_methods else tuple() - ) + def cell_methods(self, cell_methods: Iterable): + if not cell_methods: + # For backwards compatibility: Empty or null value is equivalent to (). + cell_methods = () + else: + # Can supply any iterable, which is converted (copied) to a tuple. + cell_methods = tuple(cell_methods) + for cell_method in cell_methods: + # All contents should be CellMethods. Requiring class membership is + # somewhat non-Pythonic, but simple, and not a problem for now. + if not isinstance(cell_method, iris.coords.CellMethod): + msg = ( + f"Cube.cell_methods assigned value includes {cell_method}, " + "which is not an iris.coords.CellMethod." + ) + raise ValueError(msg) + self._metadata_manager.cell_methods = cell_methods def core_data(self): """ diff --git a/lib/iris/fileformats/name_loaders.py b/lib/iris/fileformats/name_loaders.py index 3aaba3679eb..d15a3717d04 100644 --- a/lib/iris/fileformats/name_loaders.py +++ b/lib/iris/fileformats/name_loaders.py @@ -571,7 +571,9 @@ def _generate_cubes( cube.attributes[key] = value if cell_methods is not None: - cube.add_cell_method(cell_methods[i]) + cell_method = cell_methods[i] + if cell_method is not None: + cube.add_cell_method(cell_method) yield cube @@ -610,7 +612,7 @@ def _build_cell_methods(av_or_ints, coord): cell_method = None msg = "Unknown {} statistic: {!r}. Unable to create cell method." warnings.warn(msg.format(coord, av_or_int)) - cell_methods.append(cell_method) + cell_methods.append(cell_method) # NOTE: this can be a None return cell_methods diff --git a/lib/iris/tests/results/name/NAMEII_field__no_time_averaging.cml b/lib/iris/tests/results/name/NAMEII_field__no_time_averaging.cml new file mode 100644 index 00000000000..9bc2c0d1ac5 --- /dev/null +++ b/lib/iris/tests/results/name/NAMEII_field__no_time_averaging.cml @@ -0,0 +1,47 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/lib/iris/tests/results/name/NAMEII_field__no_time_averaging_0.cml b/lib/iris/tests/results/name/NAMEII_field__no_time_averaging_0.cml new file mode 100644 index 00000000000..8d1ad620d00 --- /dev/null +++ b/lib/iris/tests/results/name/NAMEII_field__no_time_averaging_0.cml @@ -0,0 +1,47 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/lib/iris/tests/test_name.py b/lib/iris/tests/test_name.py index 2843673da84..d8c5eae7462 100644 --- a/lib/iris/tests/test_name.py +++ b/lib/iris/tests/test_name.py @@ -7,9 +7,12 @@ # import iris tests first so that some things can be initialised before # importing anything else -import iris.tests as tests # isort:skip +import tempfile + import iris +import iris.tests as tests # isort:skip + @tests.skip_data class TestLoad(tests.IrisTest): @@ -39,7 +42,7 @@ def test_NAMEIII_version2(self): ) self.assertCMLApproxData(cubes, ("name", "NAMEIII_version2.cml")) - def test_NAMEII_trajectory(self): + def test_NAMEIII_trajectory(self): cubes = iris.load( tests.get_data_path(("NAME", "NAMEIII_trajectory.txt")) ) @@ -48,6 +51,32 @@ def test_NAMEII_trajectory(self): cubes, ("name", "NAMEIII_trajectory.cml"), checksum=False ) + def test_NAMEII__no_time_averaging(self): + cubes = iris.load( + tests.get_data_path(("NAME", "NAMEII_no_time_averaging.txt")) + ) + + # Also check that it saves without error. + # This was previously failing, see https://github.com/SciTools/iris/issues/3288 + with tempfile.TemporaryDirectory() as temp_dirpath: + iris.save(cubes, temp_dirpath + "/tmp.nc") + + self.assertCML( + cubes[0], + ( + "name", + "NAMEII_field__no_time_averaging_0.cml", + ), + ) + self.assertCML( + cubes, + ( + "name", + "NAMEII_field__no_time_averaging.cml", + ), + checksum=False, + ) + if __name__ == "__main__": tests.main() diff --git a/lib/iris/tests/unit/cube/test_Cube.py b/lib/iris/tests/unit/cube/test_Cube.py index e5945fb6f51..62c719aab94 100644 --- a/lib/iris/tests/unit/cube/test_Cube.py +++ b/lib/iris/tests/unit/cube/test_Cube.py @@ -9,6 +9,7 @@ # importing anything else. import iris.tests as tests # isort:skip +from collections import namedtuple from itertools import permutations from unittest import mock @@ -3109,5 +3110,83 @@ def test__repr_html__effects(simplecube, patched_cubehtml): ] +class Test__cell_methods: + @pytest.fixture(autouse=True) + def cell_measures_testdata(self): + self.cube = Cube([0]) + self.cm = CellMethod("mean", "time", "6hr") + self.cm2 = CellMethod("max", "latitude", "4hr") + + def test_none(self): + assert self.cube.cell_methods == () + + def test_one(self): + cube = Cube([0], cell_methods=[self.cm]) + expected = (self.cm,) + assert expected == cube.cell_methods + + def test_empty_assigns(self): + testargs = [(), [], {}, 0, 0.0, False, None] + results = [] + for arg in testargs: + cube = self.cube.copy() + cube.cell_methods = arg # assign test object + results.append(cube.cell_methods) # capture what is read back + expected_results = [()] * len(testargs) + assert expected_results == results + + def test_single_assigns(self): + cms = (self.cm, self.cm2) + # Any type of iterable ought to work + # But N.B. *not* testing sets, as order is not stable + testargs = [cms, list(cms), {cm: 1 for cm in cms}] + results = [] + for arg in testargs: + cube = self.cube.copy() + cube.cell_methods = arg # assign test object + results.append(cube.cell_methods) # capture what is read back + expected_results = [cms] * len(testargs) + assert expected_results == results + + def test_fail_assign_noniterable(self): + test_object = object() + with pytest.raises(TypeError, match="not iterable"): + self.cube.cell_methods = test_object + + def test_fail_create_noniterable(self): + test_object = object() + with pytest.raises(TypeError, match="not iterable"): + Cube([0], cell_methods=test_object) + + def test_fail_assign_noncellmethod(self): + test_object = object() + with pytest.raises(ValueError, match="not an iris.coords.CellMethod"): + self.cube.cell_methods = (test_object,) + + def test_fail_create_noncellmethod(self): + test_object = object() + with pytest.raises(ValueError, match="not an iris.coords.CellMethod"): + Cube([0], cell_methods=[test_object]) + + def test_assign_derivedcellmethod(self): + class DerivedCellMethod(CellMethod): + pass + + test_object = DerivedCellMethod("mean", "time", "6hr") + cms = (test_object,) + self.cube.cell_methods = (test_object,) + assert cms == self.cube.cell_methods + + def test_fail_assign_duckcellmethod(self): + # Can't currently assign a "duck-typed" CellMethod replacement, since + # implementation requires class membership (boo!) + DuckCellMethod = namedtuple("DuckCellMethod", CellMethod._names) + test_object = DuckCellMethod( + *CellMethod._names + ) # fill props with value==name + with pytest.raises(ValueError, match="not an iris.coords.CellMethod"): + self.cube.cell_methods = (test_object,) + + if __name__ == "__main__": tests.main()