Fix name loader problem

.github/workflows/ci-tests.yml

@@ -46,7 +46,7 @@ jobs:
             session: "tests"
 
     env:
-      IRIS_TEST_DATA_VERSION: "2.15"
+      IRIS_TEST_DATA_VERSION: "2.17"
       ENV_NAME: "ci-tests"
 
     steps:

docs/src/whatsnew/3.3.rst

@@ -42,8 +42,15 @@ v3.3.1 |build_date| [unreleased]
 
    The patches in this release of Iris include:
 
-#. `@pp-mo`_ fixed the Jupyter notebook display of :class:`~iris.cube.CubeList`.
-   (:issue:`4973`, :pull:`4976`)
+    #. `@pp-mo`_ fixed the Jupyter notebook display of :class:`~iris.cube.CubeList`.
+       (:issue:`4973`, :pull:`4976`)
+
+    #. `@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`).
 
 
 📢 Announcements

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):
         """

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
 
 

lib/iris/tests/results/imagerepo.json

@@ -34,6 +34,7 @@
     "gallery_tests.test_plot_wind_barbs.0": "e9e161e996169316c1fe9e96c29e36739e13c07c3d61c07f39813929c07f3f01",
     "gallery_tests.test_plot_wind_speed.0": "e9e960e996169306c1fe9e96c29e36739e03c06c3d61c07f3da139e1c07f3f01",
     "gallery_tests.test_plot_wind_speed.1": "e9e960e996169306c1ee9f96c29e36739653c06c3d61c07f39a139e1c07f3f01",
+    "gallery_tests.test_plot_zonal_means.0": "b45b3071c9a4c9a6c69c363cc327cbb3cb9634d8c9e63cf336738c6634d8c384",
     "iris.tests.experimental.test_animate.IntegrationTest.test_cube_animation.0": "fe81c17e817e3e81817e3e81857e7a817e81c17e7e81c17e7a81817e817e8c2e",
     "iris.tests.experimental.test_animate.IntegrationTest.test_cube_animation.1": "fe81857e817e7a85817e7a81857e7e817e81917a7e81817e7a81817e817e843e",
     "iris.tests.experimental.test_animate.IntegrationTest.test_cube_animation.2": "be81817ec17e7a81c17e7e81857e3e803e81817a3e81c17e7a81c17ec97e2c2f",

lib/iris/tests/results/name/NAMEII_field__no_time_averaging.cml

@@ -0,0 +1,47 @@
+<?xml version="1.0" ?>
+<cubes xmlns="urn:x-iris:cubeml-0.2">
+  <cube dtype="float32" long_name="VOLCANIC_ASH_AIR_CONCENTRATION" units="g/m2">
+    <attributes>
+      <attribute name="End of release" value="0000UTC 13/01/2011"/>
+      <attribute name="Forecast duration" value="6 hours"/>
+      <attribute name="Met data" value="NWP Flow.ECMWF ERAInt Regional"/>
+      <attribute name="NAME Version" value="NAME III (version 7.2)"/>
+      <attribute name="Quantity" value="Air Concentration"/>
+      <attribute name="Release height" value="Multiple Sources"/>
+      <attribute name="Release location" value="Multiple Sources"/>
+      <attribute name="Release rate" value="Multiple Sources"/>
+      <attribute name="Run time" value="1333UTC 07/12/2018"/>
+      <attribute name="Species" value="VOLCANIC_ASH"/>
+      <attribute name="Species Category" value="VOLCANIC"/>
+      <attribute name="Start of release" value="2200UTC 12/01/2011"/>
+      <attribute name="Time Av or Int" value="No time averaging"/>
+      <attribute name="Title" value="VA_Tutorial"/>
+    </attributes>
+    <coords>
+      <coord datadims="[0]">
+        <dimCoord bounds="[[29.9375, 30.0625],
+		[30.0625, 30.1875]]" id="77a50eb5" points="[30.0, 30.125]" shape="(2,)" standard_name="latitude" units="Unit('degrees')" value_type="float64">
+          <geogCS earth_radius="6371229.0"/>
+        </dimCoord>
+      </coord>
+      <coord datadims="[1]">
+        <dimCoord bounds="[[9.9375, 10.0625],
+		[10.0625, 10.1875]]" id="f913a8b3" points="[10.0, 10.125]" shape="(2,)" standard_name="longitude" units="Unit('degrees')" value_type="float64">
+          <geogCS earth_radius="6371229.0"/>
+        </dimCoord>
+      </coord>
+      <coord>
+        <dimCoord bounds="[[359686.0, 359686.0]]" id="cb784457" points="[359686.0]" shape="(1,)" standard_name="time" units="Unit('hours since 1970-01-01 00:00:00', calendar='standard')" value_type="float64"/>
+      </coord>
+      <coord>
+        <auxCoord id="e92ff7b7" long_name="z" points="[Vertical integral]" shape="(1,)" units="Unit('no_unit')" value_type="string">
+          <attributes>
+            <attribute name="positive" value="up"/>
+          </attributes>
+        </auxCoord>
+      </coord>
+    </coords>
+    <cellMethods/>
+    <data dtype="float32" shape="(2, 2)" state="loaded"/>
+  </cube>
+</cubes>

lib/iris/tests/results/name/NAMEII_field__no_time_averaging_0.cml

@@ -0,0 +1,47 @@
+<?xml version="1.0" ?>
+<cubes xmlns="urn:x-iris:cubeml-0.2">
+  <cube dtype="float32" long_name="VOLCANIC_ASH_AIR_CONCENTRATION" units="g/m2">
+    <attributes>
+      <attribute name="End of release" value="0000UTC 13/01/2011"/>
+      <attribute name="Forecast duration" value="6 hours"/>
+      <attribute name="Met data" value="NWP Flow.ECMWF ERAInt Regional"/>
+      <attribute name="NAME Version" value="NAME III (version 7.2)"/>
+      <attribute name="Quantity" value="Air Concentration"/>
+      <attribute name="Release height" value="Multiple Sources"/>
+      <attribute name="Release location" value="Multiple Sources"/>
+      <attribute name="Release rate" value="Multiple Sources"/>
+      <attribute name="Run time" value="1333UTC 07/12/2018"/>
+      <attribute name="Species" value="VOLCANIC_ASH"/>
+      <attribute name="Species Category" value="VOLCANIC"/>
+      <attribute name="Start of release" value="2200UTC 12/01/2011"/>
+      <attribute name="Time Av or Int" value="No time averaging"/>
+      <attribute name="Title" value="VA_Tutorial"/>
+    </attributes>
+    <coords>
+      <coord datadims="[0]">
+        <dimCoord bounds="[[29.9375, 30.0625],
+		[30.0625, 30.1875]]" id="77a50eb5" points="[30.0, 30.125]" shape="(2,)" standard_name="latitude" units="Unit('degrees')" value_type="float64">
+          <geogCS earth_radius="6371229.0"/>
+        </dimCoord>
+      </coord>
+      <coord datadims="[1]">
+        <dimCoord bounds="[[9.9375, 10.0625],
+		[10.0625, 10.1875]]" id="f913a8b3" points="[10.0, 10.125]" shape="(2,)" standard_name="longitude" units="Unit('degrees')" value_type="float64">
+          <geogCS earth_radius="6371229.0"/>
+        </dimCoord>
+      </coord>
+      <coord>
+        <dimCoord bounds="[[359686.0, 359686.0]]" id="cb784457" points="[359686.0]" shape="(1,)" standard_name="time" units="Unit('hours since 1970-01-01 00:00:00', calendar='standard')" value_type="float64"/>
+      </coord>
+      <coord>
+        <auxCoord id="e92ff7b7" long_name="z" points="[Vertical integral]" shape="(1,)" units="Unit('no_unit')" value_type="string">
+          <attributes>
+            <attribute name="positive" value="up"/>
+          </attributes>
+        </auxCoord>
+      </coord>
+    </coords>
+    <cellMethods/>
+    <data checksum="0xecbb4b55" dtype="float32" shape="(2, 2)"/>
+  </cube>
+</cubes>

lib/iris/tests/test_name.py

@@ -8,6 +8,9 @@
 # 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
 
 
@@ -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()

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
 
@@ -3048,5 +3049,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()