From 959419de9b07792575c435cc2ed364c0e354a1fc Mon Sep 17 00:00:00 2001 From: Harold Dyson Date: Tue, 2 Oct 2018 14:24:26 +0100 Subject: [PATCH 1/6] Tests for expected behaviour --- lib/iris/tests/unit/cube/test_Cube.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/iris/tests/unit/cube/test_Cube.py b/lib/iris/tests/unit/cube/test_Cube.py index 5633417c80..bcabda1df7 100644 --- a/lib/iris/tests/unit/cube/test_Cube.py +++ b/lib/iris/tests/unit/cube/test_Cube.py @@ -34,9 +34,9 @@ import iris.aux_factory import iris.coords import iris.exceptions -from iris import FUTURE from iris.analysis import WeightedAggregator, Aggregator from iris.analysis import MEAN +from iris.aux_factory import HybridHeightFactory from iris.cube import Cube from iris.coords import AuxCoord, DimCoord, CellMeasure from iris.exceptions import (CoordinateNotFoundError, CellMeasureNotFoundError, @@ -1546,6 +1546,30 @@ def test_add_cell_measure(self): cube.add_cell_measure(a_cell_measure, [0, 1]) self.assertEqual(cube.cell_measure('area'), a_cell_measure) + def test_add_valid_aux_factory(self): + cube = Cube(np.arange(8).reshape(2, 2, 2)) + delta = AuxCoord(points=[0, 1], long_name='delta', units='m') + sigma = AuxCoord(points=[0, 1], long_name='sigma') + orog = AuxCoord(np.arange(4).reshape(2, 2), units='m') + cube.add_aux_coord(delta, 0) + cube.add_aux_coord(sigma, 0) + cube.add_aux_coord(orog, (1, 2)) + factory = HybridHeightFactory(delta=delta, sigma=sigma, orography=orog) + self.assertIsNone(cube.add_aux_factory(factory)) + + def test_error_for_add_invalid_aux_factory(self): + cube = Cube(np.arange(8).reshape(2, 2, 2)) + delta = AuxCoord(points=[0, 1], long_name='delta', units='m') + sigma = AuxCoord(points=[0, 1], long_name='sigma') + orog = AuxCoord(np.arange(4).reshape(2, 2), units='m') + cube.add_aux_coord(delta, 0) + cube.add_aux_coord(sigma, 0) + # Note orography is not added to the cube here + factory = HybridHeightFactory(delta=delta, sigma=sigma, orography=orog) + expected_error = "Coordinate for factory is not present on cube" + with self.assertRaisesRegexp(ValueError, expected_error): + cube.add_aux_factory(factory) + class Test_remove_metadata(tests.IrisTest): def setUp(self): From da8dffa06303350a26832713b56b67261dc2634a Mon Sep 17 00:00:00 2001 From: Harold Dyson Date: Tue, 2 Oct 2018 14:36:33 +0100 Subject: [PATCH 2/6] Raise error if aux factory relies on a coordinate external to cube --- lib/iris/cube.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 9c42299c42..fc98a08cc2 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -994,6 +994,11 @@ def add_aux_factory(self, aux_factory): if not isinstance(aux_factory, iris.aux_factory.AuxCoordFactory): raise TypeError('Factory must be a subclass of ' 'iris.aux_factory.AuxCoordFactory.') + coords = self.coords() + for dependency in aux_factory.dependencies: + if aux_factory.dependencies[dependency] not in coords: + msg = "Coordinate for factory is not present on cube {}" + raise ValueError(msg.format(self.name())) self._aux_factories.append(aux_factory) def add_cell_measure(self, cell_measure, data_dims=None): From 92617c8492fd6099505640797ecb99364ad9fab4 Mon Sep 17 00:00:00 2001 From: Harold Dyson Date: Tue, 2 Oct 2018 14:49:12 +0100 Subject: [PATCH 3/6] Include coordinate name in error message --- lib/iris/cube.py | 9 +++++---- lib/iris/tests/unit/cube/test_Cube.py | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index fc98a08cc2..7f0fc7da5c 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -994,11 +994,12 @@ def add_aux_factory(self, aux_factory): if not isinstance(aux_factory, iris.aux_factory.AuxCoordFactory): raise TypeError('Factory must be a subclass of ' 'iris.aux_factory.AuxCoordFactory.') - coords = self.coords() + cube_coords = self.coords() for dependency in aux_factory.dependencies: - if aux_factory.dependencies[dependency] not in coords: - msg = "Coordinate for factory is not present on cube {}" - raise ValueError(msg.format(self.name())) + factory_coord = aux_factory.dependencies[dependency] + if factory_coord not in cube_coords: + msg = "{} coordinate for factory is not present on cube {}" + raise ValueError(msg.format(factory_coord.name(), self.name())) self._aux_factories.append(aux_factory) def add_cell_measure(self, cell_measure, data_dims=None): diff --git a/lib/iris/tests/unit/cube/test_Cube.py b/lib/iris/tests/unit/cube/test_Cube.py index bcabda1df7..0c88a7043c 100644 --- a/lib/iris/tests/unit/cube/test_Cube.py +++ b/lib/iris/tests/unit/cube/test_Cube.py @@ -1561,12 +1561,12 @@ def test_error_for_add_invalid_aux_factory(self): cube = Cube(np.arange(8).reshape(2, 2, 2)) delta = AuxCoord(points=[0, 1], long_name='delta', units='m') sigma = AuxCoord(points=[0, 1], long_name='sigma') - orog = AuxCoord(np.arange(4).reshape(2, 2), units='m') + orog = AuxCoord(np.arange(4).reshape(2, 2), units='m', long_name='foo') cube.add_aux_coord(delta, 0) cube.add_aux_coord(sigma, 0) # Note orography is not added to the cube here factory = HybridHeightFactory(delta=delta, sigma=sigma, orography=orog) - expected_error = "Coordinate for factory is not present on cube" + expected_error = "foo coordinate for factory is not present on cube" with self.assertRaisesRegexp(ValueError, expected_error): cube.add_aux_factory(factory) From eb4772405da4fa3a73277bd1c45715b403d14c13 Mon Sep 17 00:00:00 2001 From: Harold Dyson Date: Tue, 2 Oct 2018 16:46:08 +0100 Subject: [PATCH 4/6] Don't check whether dependencies that are "None" are in the coords. --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 7f0fc7da5c..a402b71905 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -997,7 +997,7 @@ def add_aux_factory(self, aux_factory): cube_coords = self.coords() for dependency in aux_factory.dependencies: factory_coord = aux_factory.dependencies[dependency] - if factory_coord not in cube_coords: + if factory_coord is not None and factory_coord not in cube_coords: msg = "{} coordinate for factory is not present on cube {}" raise ValueError(msg.format(factory_coord.name(), self.name())) self._aux_factories.append(aux_factory) From 054059d59b37ec2a32e3540cc155835e56df19c8 Mon Sep 17 00:00:00 2001 From: Harold Dyson Date: Wed, 3 Oct 2018 13:47:16 +0100 Subject: [PATCH 5/6] Review changes: check cube name in error and variable rename --- lib/iris/cube.py | 6 +++--- lib/iris/tests/unit/cube/test_Cube.py | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index a402b71905..ce690e93c7 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -996,10 +996,10 @@ def add_aux_factory(self, aux_factory): 'iris.aux_factory.AuxCoordFactory.') cube_coords = self.coords() for dependency in aux_factory.dependencies: - factory_coord = aux_factory.dependencies[dependency] - if factory_coord is not None and factory_coord not in cube_coords: + reference_coord = aux_factory.dependencies[dependency] + if reference_coord is not None and reference_coord not in cube_coords: msg = "{} coordinate for factory is not present on cube {}" - raise ValueError(msg.format(factory_coord.name(), self.name())) + raise ValueError(msg.format(reference_coord.name(), self.name())) self._aux_factories.append(aux_factory) def add_cell_measure(self, cell_measure, data_dims=None): diff --git a/lib/iris/tests/unit/cube/test_Cube.py b/lib/iris/tests/unit/cube/test_Cube.py index 0c88a7043c..dcc3eca7cb 100644 --- a/lib/iris/tests/unit/cube/test_Cube.py +++ b/lib/iris/tests/unit/cube/test_Cube.py @@ -1558,7 +1558,7 @@ def test_add_valid_aux_factory(self): self.assertIsNone(cube.add_aux_factory(factory)) def test_error_for_add_invalid_aux_factory(self): - cube = Cube(np.arange(8).reshape(2, 2, 2)) + cube = Cube(np.arange(8).reshape(2, 2, 2), long_name='bar') delta = AuxCoord(points=[0, 1], long_name='delta', units='m') sigma = AuxCoord(points=[0, 1], long_name='sigma') orog = AuxCoord(np.arange(4).reshape(2, 2), units='m', long_name='foo') @@ -1566,7 +1566,8 @@ def test_error_for_add_invalid_aux_factory(self): cube.add_aux_coord(sigma, 0) # Note orography is not added to the cube here factory = HybridHeightFactory(delta=delta, sigma=sigma, orography=orog) - expected_error = "foo coordinate for factory is not present on cube" + expected_error = ("foo coordinate for factory is not present on cube " + "bar") with self.assertRaisesRegexp(ValueError, expected_error): cube.add_aux_factory(factory) From a55a30d7e96dc0dc09448c8c0ffb177e375286a9 Mon Sep 17 00:00:00 2001 From: Harold Dyson Date: Wed, 3 Oct 2018 13:52:02 +0100 Subject: [PATCH 6/6] Truncated variable for flake8 --- lib/iris/cube.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index ce690e93c7..4bd947bca2 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -996,10 +996,10 @@ def add_aux_factory(self, aux_factory): 'iris.aux_factory.AuxCoordFactory.') cube_coords = self.coords() for dependency in aux_factory.dependencies: - reference_coord = aux_factory.dependencies[dependency] - if reference_coord is not None and reference_coord not in cube_coords: + ref_coord = aux_factory.dependencies[dependency] + if ref_coord is not None and ref_coord not in cube_coords: msg = "{} coordinate for factory is not present on cube {}" - raise ValueError(msg.format(reference_coord.name(), self.name())) + raise ValueError(msg.format(ref_coord.name(), self.name())) self._aux_factories.append(aux_factory) def add_cell_measure(self, cell_measure, data_dims=None):