From 5e18c6ce0ca070a06f8c74df60b2f49d243e9173 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Fri, 7 Dec 2018 11:22:26 +0000 Subject: [PATCH 01/17] ensure cubelists only contain cubes --- ...ix_2018-Dec-07_only_cubes_in_cubelists.txt | 3 + lib/iris/cube.py | 47 ++++++++++++-- lib/iris/tests/unit/cube/test_CubeList.py | 62 +++++++++++++++++++ 3 files changed, 108 insertions(+), 4 deletions(-) create mode 100644 docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt diff --git a/docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt b/docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt new file mode 100644 index 0000000000..2af5b2f537 --- /dev/null +++ b/docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt @@ -0,0 +1,3 @@ +* The `append`, `extend` and `insert` methods of :class:`iris.cube.CubeList` +now perform a check to ensure that only :class:`iris.cube.Cube` instances are +added. diff --git a/lib/iris/cube.py b/lib/iris/cube.py index f0a79d0965..eb18996991 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -156,10 +156,7 @@ def __new__(cls, list_of_cubes=None): """Given a :class:`list` of cubes, return a CubeList instance.""" cube_list = list.__new__(cls, list_of_cubes) - # Check that all items in the incoming list are cubes. Note that this - # checking does not guarantee that a CubeList instance *always* has - # just cubes in its list as the append & __getitem__ methods have not - # been overridden. + # Check that all items in the incoming list are cubes. if not all([isinstance(cube, Cube) for cube in cube_list]): raise ValueError( "All items in list_of_cubes must be Cube " "instances." @@ -210,6 +207,48 @@ def __getslice__(self, start, stop): result = CubeList(result) return result + def append(self, cube): + """ + Append a cube. + """ + if isinstance(cube, Cube): + super(CubeList, self).append(cube) + else: + raise TypeError('Only Cube instances can be appended to cubelists.' + ' Got {}.'.format(type(cube))) + + def extend(self, other_cubes): + """ + Extend cubelist by appending the cubes contained in other_cubes. + + Args: + + * other_cubes: + A cubelist or other sequence of cubes. + """ + if isinstance(other_cubes, Cube): + raise TypeError( + 'Use append to add a single cube to the cubelist.') + elif not isinstance(other_cubes, collections.Iterable): + raise TypeError( + 'Can only extend with a sequece of Cube instances.') + elif isinstance(other_cubes, CubeList) or all( + [isinstance(cube, Cube) for cube in other_cubes]): + super(CubeList, self).extend(other_cubes) + else: + raise TypeError('All items in other_cubes must be Cube ' + 'instances.') + + def insert(self, index, cube): + """ + Insert a cube before index. + """ + if isinstance(cube, Cube): + super(CubeList, self).insert(index, cube) + else: + raise TypeError('Only Cube instances can be inserted into ' + 'cubelists. Got {}.'.format(type(cube))) + def xml(self, checksum=False, order=True, byteorder=True): """Return a string of the XML that this list of cubes represents.""" diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 50c1e553bc..4958b632e6 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -24,6 +24,25 @@ import iris.tests.stock +class Test_append(tests.IrisTest): + def setUp(self): + self.cubelist = iris.cube.CubeList() + self.cube1 = iris.cube.Cube(1, long_name='foo') + self.cube2 = iris.cube.Cube(1, long_name='bar') + + def test_pass(self): + self.cubelist.append(self.cube1) + self.assertEqual(self.cubelist[-1], self.cube1) + self.cubelist.append(self.cube2) + self.assertEqual(self.cubelist[-1], self.cube2) + + def test_fail(self): + msg = ("Only Cube instances can be appended to cubelists. Got " + "") + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist.append(None) + + class Test_concatenate_cube(tests.IrisTest): def setUp(self): self.units = Unit( @@ -70,6 +89,32 @@ def test_empty(self): CubeList([]).concatenate_cube() +class Test_extend(tests.IrisTest): + def setUp(self): + self.cube1 = iris.cube.Cube(1, long_name='foo') + self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cubelist1 = iris.cube.CubeList([self.cube1]) + self.cubelist2 = iris.cube.CubeList([self.cube2]) + + def test_pass(self): + cubelist = self.cubelist1.copy() + cubelist.extend(self.cubelist2) + self.assertEqual(cubelist, self.cubelist1 + self.cubelist2) + cubelist.extend([self.cube2]) + self.assertEqual(cubelist[-1], self.cube2) + + def test_fail(self): + msg = 'Use append to add a single cube to the cubelist.' + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist1.extend(self.cube1) + msg = 'Can only extend with a sequece of Cube instances.' + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist1.extend(None) + msg = 'All items in other_cubes must be Cube instances.' + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist1.extend(range(3)) + + class Test_extract_overlapping(tests.IrisTest): def setUp(self): shape = (6, 14, 19) @@ -130,6 +175,23 @@ def test_different_orders(self): self.assertEqual(b.coord("time"), self.cube.coord("time")[2:4]) +class Test_insert(tests.IrisTest): + def setUp(self): + self.cube1 = iris.cube.Cube(1, long_name='foo') + self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cubelist = iris.cube.CubeList([self.cube1] * 3) + + def test_pass(self): + self.cubelist.insert(1, self.cube2) + self.assertEqual(self.cubelist[1], self.cube2) + + def test_fail(self): + msg = ("Only Cube instances can be inserted into cubelists. Got " + "") + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist.insert(0, None) + + class Test_merge_cube(tests.IrisTest): def setUp(self): self.cube1 = Cube([1, 2, 3], "air_temperature", units="K") From 3ed9de59ef5a456060dce609cf6298581aecee60 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Wed, 12 Dec 2018 17:47:41 +0000 Subject: [PATCH 02/17] address __iadd__ and __setitem__ --- lib/iris/cube.py | 28 +++++++++++++++++++--- lib/iris/tests/unit/cube/test_CubeList.py | 29 +++++++++++++++++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index eb18996991..5f9b9d72f5 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -207,6 +207,28 @@ def __getslice__(self, start, stop): result = CubeList(result) return result + def __iadd__(self, other_cubes): + """ + Add a sequence of cubes to the cubelist in place. + """ + if isinstance(other_cubes, Cube) or not isinstance( + other_cubes, collections.Iterable): + raise TypeError( + 'Can only add a sequence of Cube instances.') + elif isinstance(other_cubes, CubeList) or all( + [isinstance(cube, Cube) for cube in other_cubes]): + super(CubeList, self).__iadd__(other_cubes) + else: + raise ValueError('All items in added sequence must be Cube ' + 'instances.') + + def __setitem__(self, key, cube): + """Set self[key] to cube""" + if isinstance(cube, Cube): + super(CubeList, self).__setitem__(key, cube) + else: + raise TypeError('Elements of cubelists must be Cube instances') + def append(self, cube): """ Append a cube. @@ -231,13 +253,13 @@ def extend(self, other_cubes): 'Use append to add a single cube to the cubelist.') elif not isinstance(other_cubes, collections.Iterable): raise TypeError( - 'Can only extend with a sequece of Cube instances.') + 'Can only extend with a sequence of Cube instances.') elif isinstance(other_cubes, CubeList) or all( [isinstance(cube, Cube) for cube in other_cubes]): super(CubeList, self).extend(other_cubes) else: - raise TypeError('All items in other_cubes must be Cube ' - 'instances.') + raise ValueError('All items in other_cubes must be Cube ' + 'instances.') def insert(self, index, cube): """ diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 4958b632e6..9da2bbe064 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -107,11 +107,11 @@ def test_fail(self): msg = 'Use append to add a single cube to the cubelist.' with self.assertRaisesRegexp(TypeError, msg): self.cubelist1.extend(self.cube1) - msg = 'Can only extend with a sequece of Cube instances.' + msg = 'Can only extend with a sequence of Cube instances.' with self.assertRaisesRegexp(TypeError, msg): self.cubelist1.extend(None) msg = 'All items in other_cubes must be Cube instances.' - with self.assertRaisesRegexp(TypeError, msg): + with self.assertRaisesRegexp(ValueError, msg): self.cubelist1.extend(range(3)) @@ -175,6 +175,31 @@ def test_different_orders(self): self.assertEqual(b.coord("time"), self.cube.coord("time")[2:4]) +class Test_iadd(tests.IrisTest): + def setUp(self): + self.cube1 = iris.cube.Cube(1, long_name='foo') + self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cubelist1 = iris.cube.CubeList([self.cube1]) + self.cubelist2 = iris.cube.CubeList([self.cube2]) + + def test_pass(self): + cubelist = self.cubelist1.copy() + cubelist += self.cubelist2 + self.assertEqual(cubelist, self.cubelist1 + self.cubelist2) + cubelist += [self.cube2] + self.assertEqual(cubelist[-1], self.cube2) + + def test_fail(self): + msg = 'Can only add a sequence of Cube instances.' + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist1 += self.cube1 + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist1 += 1. + msg = 'All items in added sequence must be Cube instances.' + with self.assertRaisesRegexp(ValueError, msg): + self.cubelist1 += range(3) + + class Test_insert(tests.IrisTest): def setUp(self): self.cube1 = iris.cube.Cube(1, long_name='foo') From 1c10366e8ab4d4af92aae0b789aa800e51795452 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Wed, 12 Dec 2018 18:01:06 +0000 Subject: [PATCH 03/17] __setitem__ tests --- lib/iris/cube.py | 3 ++- lib/iris/tests/unit/cube/test_CubeList.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 5f9b9d72f5..f782f31cf4 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -227,7 +227,8 @@ def __setitem__(self, key, cube): if isinstance(cube, Cube): super(CubeList, self).__setitem__(key, cube) else: - raise TypeError('Elements of cubelists must be Cube instances') + raise TypeError('Elements of cubelists must be Cube instances. ' + 'Got {}.'.format(type(cube))) def append(self, cube): """ diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 9da2bbe064..ee42d35d2c 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -361,6 +361,23 @@ def test_combination_with_extra_triple(self): self.assertCML(cube, checksum=False) +class Test_setitem(tests.IrisTest): + def setUp(self): + self.cube1 = iris.cube.Cube(1, long_name='foo') + self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cubelist = iris.cube.CubeList([self.cube1] * 3) + + def test_pass(self): + self.cubelist[1] = self.cube2 + self.assertEqual(self.cubelist[1], self.cube2) + + def test_fail(self): + msg = ("Elements of cubelists must be Cube instances. Got " + ".") + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist[0] = None + + class Test_xml(tests.IrisTest): def setUp(self): self.cubes = CubeList([Cube(np.arange(3)), Cube(np.arange(3))]) From d05d69796567b0575220462193c47022374bd77d Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Thu, 13 Dec 2018 11:47:31 +0000 Subject: [PATCH 04/17] test for setting more than 1 item --- lib/iris/tests/unit/cube/test_CubeList.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index ee42d35d2c..8dc83a2c2a 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -365,17 +365,24 @@ class Test_setitem(tests.IrisTest): def setUp(self): self.cube1 = iris.cube.Cube(1, long_name='foo') self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cube3 = iris.cube.Cube(1, long_name='boo') self.cubelist = iris.cube.CubeList([self.cube1] * 3) def test_pass(self): self.cubelist[1] = self.cube2 self.assertEqual(self.cubelist[1], self.cube2) + self.cubelist[:2] = (self.cube2, self.cube3) + self.assertEqual( + self.cubelist, + iris.cube.Cubelist([self.cube2, self.cube3, self.cube1])) def test_fail(self): msg = ("Elements of cubelists must be Cube instances. Got " ".") with self.assertRaisesRegexp(TypeError, msg): self.cubelist[0] = None + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist[0:2] = [self.cube3, None] class Test_xml(tests.IrisTest): From 5ab42beaf8276b5dbd70d9ec673ca0ffa62e0e0d Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Fri, 14 Dec 2018 18:42:42 +0000 Subject: [PATCH 05/17] Fix __setitem__ and Py2 tweaks --- lib/iris/cube.py | 23 ++++++++++++++------ lib/iris/tests/unit/cube/test_CubeList.py | 26 ++++++++++++++--------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index f782f31cf4..8dd0cb8ab8 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -222,13 +222,24 @@ def __iadd__(self, other_cubes): raise ValueError('All items in added sequence must be Cube ' 'instances.') - def __setitem__(self, key, cube): - """Set self[key] to cube""" - if isinstance(cube, Cube): - super(CubeList, self).__setitem__(key, cube) + def __setitem__(self, key, cube_or_sequence): + """Set self[key] to cube or sequence of cubes""" + if isinstance(key, int): + if not isinstance(cube_or_sequence, Cube): + raise TypeError('Elements of cubelists must be Cube instances.' + ' Got {}.'.format(type(cube_or_sequence))) else: - raise TypeError('Elements of cubelists must be Cube instances. ' - 'Got {}.'.format(type(cube))) + # key is a slice (or exception will come from list method). + if isinstance(cube_or_sequence, Cube) or not isinstance( + cube_or_sequence, collections.Iterable): + raise TypeError('Assigning to a slice of a cubelist requires ' + 'a sequence of cubes.') + elif not all([isinstance(cube, Cube) for + cube in cube_or_sequence]): + raise ValueError( + 'Elements of cubelists must be Cube instances.') + + super(CubeList, self).__setitem__(key, cube_or_sequence) def append(self, cube): """ diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 8dc83a2c2a..7ca8692ecc 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -12,6 +12,8 @@ import collections from unittest import mock +import copy + from cf_units import Unit import numpy as np @@ -37,8 +39,7 @@ def test_pass(self): self.assertEqual(self.cubelist[-1], self.cube2) def test_fail(self): - msg = ("Only Cube instances can be appended to cubelists. Got " - "") + msg = "Only Cube instances can be appended to cubelists. Got " with self.assertRaisesRegexp(TypeError, msg): self.cubelist.append(None) @@ -97,7 +98,7 @@ def setUp(self): self.cubelist2 = iris.cube.CubeList([self.cube2]) def test_pass(self): - cubelist = self.cubelist1.copy() + cubelist = copy.copy(self.cubelist1) cubelist.extend(self.cubelist2) self.assertEqual(cubelist, self.cubelist1 + self.cubelist2) cubelist.extend([self.cube2]) @@ -183,7 +184,7 @@ def setUp(self): self.cubelist2 = iris.cube.CubeList([self.cube2]) def test_pass(self): - cubelist = self.cubelist1.copy() + cubelist = copy.copy(self.cubelist1) cubelist += self.cubelist2 self.assertEqual(cubelist, self.cubelist1 + self.cubelist2) cubelist += [self.cube2] @@ -211,8 +212,7 @@ def test_pass(self): self.assertEqual(self.cubelist[1], self.cube2) def test_fail(self): - msg = ("Only Cube instances can be inserted into cubelists. Got " - "") + msg = "Only Cube instances can be inserted into cubelists. Got " with self.assertRaisesRegexp(TypeError, msg): self.cubelist.insert(0, None) @@ -374,15 +374,21 @@ def test_pass(self): self.cubelist[:2] = (self.cube2, self.cube3) self.assertEqual( self.cubelist, - iris.cube.Cubelist([self.cube2, self.cube3, self.cube1])) + iris.cube.CubeList([self.cube2, self.cube3, self.cube1])) def test_fail(self): - msg = ("Elements of cubelists must be Cube instances. Got " - ".") + msg = "Elements of cubelists must be Cube instances. Got " with self.assertRaisesRegexp(TypeError, msg): self.cubelist[0] = None - with self.assertRaisesRegexp(TypeError, msg): + msg = "Elements of cubelists must be Cube instances." + with self.assertRaisesRegexp(ValueError, msg): self.cubelist[0:2] = [self.cube3, None] + msg = ('Assigning to a slice of a cubelist requires a sequence of ' + 'cubes.') + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist[:1] = 2.5 + with self.assertRaisesRegexp(TypeError, msg): + self.cubelist[:1] = self.cube1 class Test_xml(tests.IrisTest): From cd4d873f57b30f0160043cedd760958521ba327e Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Mon, 17 Dec 2018 11:32:32 +0000 Subject: [PATCH 06/17] implement __setslice__ for Python2.7 --- lib/iris/cube.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 8dd0cb8ab8..4badc10680 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -241,6 +241,18 @@ def __setitem__(self, key, cube_or_sequence): super(CubeList, self).__setitem__(key, cube_or_sequence) + # __setslice__ is only required for python2.7 compatibility. + def __setslice__(self, *args): + cubes = args[-1] + if isinstance(cubes, Cube) or not isinstance( + cubes, collections.Iterable): + raise TypeError('Assigning to a slice of a cubelist requires ' + 'a sequence of cubes.') + elif not all([isinstance(cube, Cube) for cube in cubes]): + raise ValueError('Elements of cubelists must be Cube instances.') + + super(CubeList, self).__setslice__(*args) + def append(self, cube): """ Append a cube. From 21c8d6acd856fc046c0da98063c7600d385f182b Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Wed, 6 Feb 2019 11:50:30 +0000 Subject: [PATCH 07/17] change exceptions to warnings --- lib/iris/cube.py | 85 ++++++++++------------- lib/iris/tests/unit/cube/test_CubeList.py | 43 ++++++------ 2 files changed, 57 insertions(+), 71 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 4badc10680..990c16c9db 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -185,7 +185,30 @@ def _repr_html_(self): representer = CubeListRepresentation(self) return representer.repr_html() + def _check_iscube(self, obj): + """ + Raise a warning if obj is not a cube. + """ + if not isinstance(obj, Cube): + msg = ('Cubelist now contains object of type {}. This may ' + 'adversely affect subsequent operations.') + warnings.warn(msg.format(type(obj))) + + def _check_cube_sequence(self, sequence): + """ + Raise one or more warnings if sequence contains elements that are not + Cubes. Skip this if the sequence is a CubeList, as we can assume it + was already checked. + """ + if (isinstance(sequence, collections.Iterable) and + not isinstance(sequence, Cube) and + not isinstance(sequence, CubeList)): + for obj in sequence: + self._check_iscube(obj) + + # TODO #370 Which operators need overloads? + def __add__(self, other): return CubeList(list.__add__(self, other)) @@ -211,57 +234,32 @@ def __iadd__(self, other_cubes): """ Add a sequence of cubes to the cubelist in place. """ - if isinstance(other_cubes, Cube) or not isinstance( - other_cubes, collections.Iterable): - raise TypeError( - 'Can only add a sequence of Cube instances.') - elif isinstance(other_cubes, CubeList) or all( - [isinstance(cube, Cube) for cube in other_cubes]): - super(CubeList, self).__iadd__(other_cubes) - else: - raise ValueError('All items in added sequence must be Cube ' - 'instances.') + self._check_cube_sequence(other_cubes) + super(CubeList, self).__iadd__(other_cubes) def __setitem__(self, key, cube_or_sequence): """Set self[key] to cube or sequence of cubes""" if isinstance(key, int): - if not isinstance(cube_or_sequence, Cube): - raise TypeError('Elements of cubelists must be Cube instances.' - ' Got {}.'.format(type(cube_or_sequence))) + # should have single cube. + self._check_iscube(cube_or_sequence) else: # key is a slice (or exception will come from list method). - if isinstance(cube_or_sequence, Cube) or not isinstance( - cube_or_sequence, collections.Iterable): - raise TypeError('Assigning to a slice of a cubelist requires ' - 'a sequence of cubes.') - elif not all([isinstance(cube, Cube) for - cube in cube_or_sequence]): - raise ValueError( - 'Elements of cubelists must be Cube instances.') + self._check_cube_sequence(cube_or_sequence) super(CubeList, self).__setitem__(key, cube_or_sequence) # __setslice__ is only required for python2.7 compatibility. def __setslice__(self, *args): cubes = args[-1] - if isinstance(cubes, Cube) or not isinstance( - cubes, collections.Iterable): - raise TypeError('Assigning to a slice of a cubelist requires ' - 'a sequence of cubes.') - elif not all([isinstance(cube, Cube) for cube in cubes]): - raise ValueError('Elements of cubelists must be Cube instances.') - + self._check_cube_sequence(cubes) super(CubeList, self).__setslice__(*args) def append(self, cube): """ Append a cube. """ - if isinstance(cube, Cube): - super(CubeList, self).append(cube) - else: - raise TypeError('Only Cube instances can be appended to cubelists.' - ' Got {}.'.format(type(cube))) + self._check_iscube(cube) + super(CubeList, self).append(cube) def extend(self, other_cubes): """ @@ -272,28 +270,15 @@ def extend(self, other_cubes): * other_cubes: A cubelist or other sequence of cubes. """ - if isinstance(other_cubes, Cube): - raise TypeError( - 'Use append to add a single cube to the cubelist.') - elif not isinstance(other_cubes, collections.Iterable): - raise TypeError( - 'Can only extend with a sequence of Cube instances.') - elif isinstance(other_cubes, CubeList) or all( - [isinstance(cube, Cube) for cube in other_cubes]): - super(CubeList, self).extend(other_cubes) - else: - raise ValueError('All items in other_cubes must be Cube ' - 'instances.') + self._check_cube_sequence(other_cubes) + super(CubeList, self).extend(other_cubes) def insert(self, index, cube): """ Insert a cube before index. """ - if isinstance(cube, Cube): - super(CubeList, self).insert(index, cube) - else: - raise TypeError('Only Cube instances can be inserted into ' - 'cubelists. Got {}.'.format(type(cube))) + self._check_iscube(cube) + super(CubeList, self).insert(index, cube) def xml(self, checksum=False, order=True, byteorder=True): """Return a string of the XML that this list of cubes represents.""" diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 7ca8692ecc..be0794a533 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -25,6 +25,8 @@ from iris.fileformats.pp import STASH import iris.tests.stock +NOT_CUBE_MSG = 'Cubelist now contains object of type ' + class Test_append(tests.IrisTest): def setUp(self): @@ -38,9 +40,8 @@ def test_pass(self): self.cubelist.append(self.cube2) self.assertEqual(self.cubelist[-1], self.cube2) - def test_fail(self): - msg = "Only Cube instances can be appended to cubelists. Got " - with self.assertRaisesRegexp(TypeError, msg): + def test_warn(self): + with self.assertWarnsRegexp(NOT_CUBE_MSG): self.cubelist.append(None) @@ -105,14 +106,14 @@ def test_pass(self): self.assertEqual(cubelist[-1], self.cube2) def test_fail(self): - msg = 'Use append to add a single cube to the cubelist.' - with self.assertRaisesRegexp(TypeError, msg): + with self.assertRaisesRegexp(TypeError, 'Cube is not iterable'): self.cubelist1.extend(self.cube1) - msg = 'Can only extend with a sequence of Cube instances.' + msg = "'NoneType' object is not iterable" with self.assertRaisesRegexp(TypeError, msg): self.cubelist1.extend(None) - msg = 'All items in other_cubes must be Cube instances.' - with self.assertRaisesRegexp(ValueError, msg): + + def test_warn(self): + with self.assertWarnsRegexp(NOT_CUBE_MSG): self.cubelist1.extend(range(3)) @@ -191,13 +192,15 @@ def test_pass(self): self.assertEqual(cubelist[-1], self.cube2) def test_fail(self): - msg = 'Can only add a sequence of Cube instances.' + msg = 'Cube is not iterable' with self.assertRaisesRegexp(TypeError, msg): self.cubelist1 += self.cube1 + msg = "'float' object is not iterable" with self.assertRaisesRegexp(TypeError, msg): self.cubelist1 += 1. - msg = 'All items in added sequence must be Cube instances.' - with self.assertRaisesRegexp(ValueError, msg): + + def test_warn(self): + with self.assertWarnsRegexp(NOT_CUBE_MSG): self.cubelist1 += range(3) @@ -211,9 +214,8 @@ def test_pass(self): self.cubelist.insert(1, self.cube2) self.assertEqual(self.cubelist[1], self.cube2) - def test_fail(self): - msg = "Only Cube instances can be inserted into cubelists. Got " - with self.assertRaisesRegexp(TypeError, msg): + def test_warn(self): + with self.assertWarnsRegexp(NOT_CUBE_MSG): self.cubelist.insert(0, None) @@ -376,15 +378,14 @@ def test_pass(self): self.cubelist, iris.cube.CubeList([self.cube2, self.cube3, self.cube1])) - def test_fail(self): - msg = "Elements of cubelists must be Cube instances. Got " - with self.assertRaisesRegexp(TypeError, msg): + def test_warn(self): + with self.assertWarnsRegexp(NOT_CUBE_MSG): self.cubelist[0] = None - msg = "Elements of cubelists must be Cube instances." - with self.assertRaisesRegexp(ValueError, msg): + with self.assertWarnsRegexp(NOT_CUBE_MSG): self.cubelist[0:2] = [self.cube3, None] - msg = ('Assigning to a slice of a cubelist requires a sequence of ' - 'cubes.') + + def test_fail(self): + msg = "can only assign an iterable" with self.assertRaisesRegexp(TypeError, msg): self.cubelist[:1] = 2.5 with self.assertRaisesRegexp(TypeError, msg): From a50386dd4d4f3a317228edcb0d565b1054c2ef11 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Wed, 6 Feb 2019 11:54:10 +0000 Subject: [PATCH 08/17] stickler --- lib/iris/cube.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 990c16c9db..e3e02ff77d 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -201,12 +201,11 @@ def _check_cube_sequence(self, sequence): was already checked. """ if (isinstance(sequence, collections.Iterable) and - not isinstance(sequence, Cube) and - not isinstance(sequence, CubeList)): + not isinstance(sequence, Cube) and + not isinstance(sequence, CubeList)): for obj in sequence: self._check_iscube(obj) - # TODO #370 Which operators need overloads? def __add__(self, other): From 59f0302765444f3c2584a7b7a2d3b0bc5f9c0ca7 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Fri, 8 Feb 2019 09:28:33 +0000 Subject: [PATCH 09/17] duck type check; move helpers outside class --- lib/iris/cube.py | 63 ++++++++++------------- lib/iris/tests/unit/cube/test_CubeList.py | 14 ++--- 2 files changed, 35 insertions(+), 42 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index e3e02ff77d..4592883d70 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -144,6 +144,26 @@ def merged(self, unique=False): [pair.merged(unique) for pair in self.pairs] ) +def _check_iscube(obj): + """ + Raise a warning if obj does not look like a cube. + """ + if not hasattr(obj, 'add_aux_coord'): + msg = ("Cubelist now contains object of type '{}'. This may " + "adversely affect subsequent operations.") + warnings.warn(msg.format(type(obj).__name__)) + +def _check_cube_sequence(sequence): + """ + Raise one or more warnings if sequence contains elements that are not + Cubes (or cube-like). Skip this if the sequence is a CubeList, as we can + assume it was already checked. + """ + if (isinstance(sequence, collections.Iterable) and + not isinstance(sequence, Cube) and + not isinstance(sequence, CubeList)): + for obj in sequence: + _check_iscube(obj) class CubeList(list): """ @@ -177,34 +197,7 @@ def __str__(self): def __repr__(self): """Runs repr on every cube.""" - return "[%s]" % ",\n".join([repr(cube) for cube in self]) - - def _repr_html_(self): - from iris.experimental.representation import CubeListRepresentation - - representer = CubeListRepresentation(self) - return representer.repr_html() - - def _check_iscube(self, obj): - """ - Raise a warning if obj is not a cube. - """ - if not isinstance(obj, Cube): - msg = ('Cubelist now contains object of type {}. This may ' - 'adversely affect subsequent operations.') - warnings.warn(msg.format(type(obj))) - - def _check_cube_sequence(self, sequence): - """ - Raise one or more warnings if sequence contains elements that are not - Cubes. Skip this if the sequence is a CubeList, as we can assume it - was already checked. - """ - if (isinstance(sequence, collections.Iterable) and - not isinstance(sequence, Cube) and - not isinstance(sequence, CubeList)): - for obj in sequence: - self._check_iscube(obj) + return '[%s]' % ',\n'.join([repr(cube) for cube in self]) # TODO #370 Which operators need overloads? @@ -233,31 +226,31 @@ def __iadd__(self, other_cubes): """ Add a sequence of cubes to the cubelist in place. """ - self._check_cube_sequence(other_cubes) + _check_cube_sequence(other_cubes) super(CubeList, self).__iadd__(other_cubes) def __setitem__(self, key, cube_or_sequence): """Set self[key] to cube or sequence of cubes""" if isinstance(key, int): # should have single cube. - self._check_iscube(cube_or_sequence) + _check_iscube(cube_or_sequence) else: # key is a slice (or exception will come from list method). - self._check_cube_sequence(cube_or_sequence) + _check_cube_sequence(cube_or_sequence) super(CubeList, self).__setitem__(key, cube_or_sequence) # __setslice__ is only required for python2.7 compatibility. def __setslice__(self, *args): cubes = args[-1] - self._check_cube_sequence(cubes) + _check_cube_sequence(cubes) super(CubeList, self).__setslice__(*args) def append(self, cube): """ Append a cube. """ - self._check_iscube(cube) + _check_iscube(cube) super(CubeList, self).append(cube) def extend(self, other_cubes): @@ -269,14 +262,14 @@ def extend(self, other_cubes): * other_cubes: A cubelist or other sequence of cubes. """ - self._check_cube_sequence(other_cubes) + _check_cube_sequence(other_cubes) super(CubeList, self).extend(other_cubes) def insert(self, index, cube): """ Insert a cube before index. """ - self._check_iscube(cube) + _check_iscube(cube) super(CubeList, self).insert(index, cube) def xml(self, checksum=False, order=True, byteorder=True): diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index be0794a533..59f51e585b 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -25,7 +25,7 @@ from iris.fileformats.pp import STASH import iris.tests.stock -NOT_CUBE_MSG = 'Cubelist now contains object of type ' +NOT_CUBE_MSG = "Cubelist now contains object of type '{}'" class Test_append(tests.IrisTest): @@ -41,7 +41,7 @@ def test_pass(self): self.assertEqual(self.cubelist[-1], self.cube2) def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG): + with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): self.cubelist.append(None) @@ -113,7 +113,7 @@ def test_fail(self): self.cubelist1.extend(None) def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG): + with self.assertWarnsRegexp(NOT_CUBE_MSG.format('int')): self.cubelist1.extend(range(3)) @@ -200,7 +200,7 @@ def test_fail(self): self.cubelist1 += 1. def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG): + with self.assertWarnsRegexp(NOT_CUBE_MSG.format('int')): self.cubelist1 += range(3) @@ -215,7 +215,7 @@ def test_pass(self): self.assertEqual(self.cubelist[1], self.cube2) def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG): + with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): self.cubelist.insert(0, None) @@ -379,9 +379,9 @@ def test_pass(self): iris.cube.CubeList([self.cube2, self.cube3, self.cube1])) def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG): + with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): self.cubelist[0] = None - with self.assertWarnsRegexp(NOT_CUBE_MSG): + with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): self.cubelist[0:2] = [self.cube3, None] def test_fail(self): From 2c348a483e13f582d882d5029cfafcbf54a183af Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Fri, 8 Feb 2019 09:30:56 +0000 Subject: [PATCH 10/17] blank lines --- lib/iris/cube.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 4592883d70..a7764eca77 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -144,6 +144,7 @@ def merged(self, unique=False): [pair.merged(unique) for pair in self.pairs] ) + def _check_iscube(obj): """ Raise a warning if obj does not look like a cube. @@ -153,6 +154,7 @@ def _check_iscube(obj): "adversely affect subsequent operations.") warnings.warn(msg.format(type(obj).__name__)) + def _check_cube_sequence(sequence): """ Raise one or more warnings if sequence contains elements that are not @@ -165,6 +167,7 @@ def _check_cube_sequence(sequence): for obj in sequence: _check_iscube(obj) + class CubeList(list): """ All the functionality of a standard :class:`list` with added "Cube" From eb2f94492e5be2b9cea2a47ae8e48311b5cde7a0 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Fri, 8 Feb 2019 10:21:01 +0000 Subject: [PATCH 11/17] proposed: revert warnings to exceptions --- lib/iris/cube.py | 19 ++++++++------- lib/iris/tests/unit/cube/test_CubeList.py | 28 +++++++++++------------ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index a7764eca77..f9608e088f 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -147,25 +147,28 @@ def merged(self, unique=False): def _check_iscube(obj): """ - Raise a warning if obj does not look like a cube. + Raise an exception if obj does not look like a cube. """ if not hasattr(obj, 'add_aux_coord'): - msg = ("Cubelist now contains object of type '{}'. This may " - "adversely affect subsequent operations.") - warnings.warn(msg.format(type(obj).__name__)) + msg = ("Object of type '{}' does not look like a cube so can't be " + "included in cubelist.") + raise TypeError(msg.format(type(obj).__name__)) def _check_cube_sequence(sequence): """ - Raise one or more warnings if sequence contains elements that are not - Cubes (or cube-like). Skip this if the sequence is a CubeList, as we can - assume it was already checked. + Raise an exception if sequence contains an element that is not a Cube (or + cube-like). Skip this if the sequence is a CubeList, as we can assume it + was already checked. """ if (isinstance(sequence, collections.Iterable) and not isinstance(sequence, Cube) and not isinstance(sequence, CubeList)): for obj in sequence: - _check_iscube(obj) + if not hasattr(obj, 'add_aux_coord'): + msg = ("Sequence contains object of type '{}', which does not " + "look like a cube so can't be included in cubelist.") + raise ValueError(msg.format(type(obj).__name__)) class CubeList(list): diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 59f51e585b..4b9acd03d9 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -25,7 +25,7 @@ from iris.fileformats.pp import STASH import iris.tests.stock -NOT_CUBE_MSG = "Cubelist now contains object of type '{}'" +NOT_CUBE_MSG = "bject of type '{}'" class Test_append(tests.IrisTest): @@ -40,8 +40,9 @@ def test_pass(self): self.cubelist.append(self.cube2) self.assertEqual(self.cubelist[-1], self.cube2) - def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): + def test_fail(self): + with self.assertRaisesRegexp(TypeError, + NOT_CUBE_MSG.format('NoneType')): self.cubelist.append(None) @@ -111,9 +112,7 @@ def test_fail(self): msg = "'NoneType' object is not iterable" with self.assertRaisesRegexp(TypeError, msg): self.cubelist1.extend(None) - - def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG.format('int')): + with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('int')): self.cubelist1.extend(range(3)) @@ -198,9 +197,7 @@ def test_fail(self): msg = "'float' object is not iterable" with self.assertRaisesRegexp(TypeError, msg): self.cubelist1 += 1. - - def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG.format('int')): + with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('int')): self.cubelist1 += range(3) @@ -214,8 +211,9 @@ def test_pass(self): self.cubelist.insert(1, self.cube2) self.assertEqual(self.cubelist[1], self.cube2) - def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): + def test_fail(self): + with self.assertRaisesRegexp(TypeError, + NOT_CUBE_MSG.format('NoneType')): self.cubelist.insert(0, None) @@ -378,10 +376,12 @@ def test_pass(self): self.cubelist, iris.cube.CubeList([self.cube2, self.cube3, self.cube1])) - def test_warn(self): - with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): + def test_fail(self): + with self.assertRaisesRegexp(TypeError, + NOT_CUBE_MSG.format('NoneType')): self.cubelist[0] = None - with self.assertWarnsRegexp(NOT_CUBE_MSG.format('NoneType')): + with self.assertRaisesRegexp(ValueError, + NOT_CUBE_MSG.format('NoneType')): self.cubelist[0:2] = [self.cube3, None] def test_fail(self): From 2239915ea33ce09ffce9be910638a21cbfb4e512 Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Fri, 8 Feb 2019 10:23:47 +0000 Subject: [PATCH 12/17] remove stray extra 'test_fail' --- lib/iris/tests/unit/cube/test_CubeList.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 4b9acd03d9..508e0eff96 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -384,7 +384,6 @@ def test_fail(self): NOT_CUBE_MSG.format('NoneType')): self.cubelist[0:2] = [self.cube3, None] - def test_fail(self): msg = "can only assign an iterable" with self.assertRaisesRegexp(TypeError, msg): self.cubelist[:1] = 2.5 From 02447b39bd54b1c7a59306af23c75b7b8ff3960f Mon Sep 17 00:00:00 2001 From: Ruth Comer Date: Wed, 20 Feb 2019 18:03:44 +0000 Subject: [PATCH 13/17] pass sequences through __init__; _assert_is_cube --- lib/iris/cube.py | 52 +++++++---------------- lib/iris/tests/unit/cube/test_CubeList.py | 8 ++-- 2 files changed, 19 insertions(+), 41 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index f9608e088f..d25d2a574e 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -145,32 +145,6 @@ def merged(self, unique=False): ) -def _check_iscube(obj): - """ - Raise an exception if obj does not look like a cube. - """ - if not hasattr(obj, 'add_aux_coord'): - msg = ("Object of type '{}' does not look like a cube so can't be " - "included in cubelist.") - raise TypeError(msg.format(type(obj).__name__)) - - -def _check_cube_sequence(sequence): - """ - Raise an exception if sequence contains an element that is not a Cube (or - cube-like). Skip this if the sequence is a CubeList, as we can assume it - was already checked. - """ - if (isinstance(sequence, collections.Iterable) and - not isinstance(sequence, Cube) and - not isinstance(sequence, CubeList)): - for obj in sequence: - if not hasattr(obj, 'add_aux_coord'): - msg = ("Sequence contains object of type '{}', which does not " - "look like a cube so can't be included in cubelist.") - raise ValueError(msg.format(type(obj).__name__)) - - class CubeList(list): """ All the functionality of a standard :class:`list` with added "Cube" @@ -205,6 +179,12 @@ def __repr__(self): """Runs repr on every cube.""" return '[%s]' % ',\n'.join([repr(cube) for cube in self]) + @staticmethod + def _assert_is_cube(obj): + if not isinstance(obj, Cube): + msg = ("Object of type '{}' does not belong in a cubelist.") + raise ValueError(msg.format(type(obj).__name__)) + # TODO #370 Which operators need overloads? def __add__(self, other): @@ -232,31 +212,30 @@ def __iadd__(self, other_cubes): """ Add a sequence of cubes to the cubelist in place. """ - _check_cube_sequence(other_cubes) - super(CubeList, self).__iadd__(other_cubes) + super(CubeList, self).__iadd__(CubeList(other_cubes)) def __setitem__(self, key, cube_or_sequence): """Set self[key] to cube or sequence of cubes""" if isinstance(key, int): # should have single cube. - _check_iscube(cube_or_sequence) + self._assert_is_cube(cube_or_sequence) else: # key is a slice (or exception will come from list method). - _check_cube_sequence(cube_or_sequence) + cube_or_sequence = CubeList(cube_or_sequence) super(CubeList, self).__setitem__(key, cube_or_sequence) # __setslice__ is only required for python2.7 compatibility. def __setslice__(self, *args): - cubes = args[-1] - _check_cube_sequence(cubes) - super(CubeList, self).__setslice__(*args) + args_list = list(args) + args_list[-1] = CubeList(args[-1]) + super(CubeList, self).__setslice__(*args_list) def append(self, cube): """ Append a cube. """ - _check_iscube(cube) + self._assert_is_cube(cube) super(CubeList, self).append(cube) def extend(self, other_cubes): @@ -268,14 +247,13 @@ def extend(self, other_cubes): * other_cubes: A cubelist or other sequence of cubes. """ - _check_cube_sequence(other_cubes) - super(CubeList, self).extend(other_cubes) + super(CubeList, self).extend(CubeList(other_cubes)) def insert(self, index, cube): """ Insert a cube before index. """ - _check_iscube(cube) + self._assert_is_cube(cube) super(CubeList, self).insert(index, cube) def xml(self, checksum=False, order=True, byteorder=True): diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 508e0eff96..9a7ad8ad9f 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -25,7 +25,7 @@ from iris.fileformats.pp import STASH import iris.tests.stock -NOT_CUBE_MSG = "bject of type '{}'" +NOT_CUBE_MSG = "Object of type '{}' does not belong in a cubelist." class Test_append(tests.IrisTest): @@ -41,7 +41,7 @@ def test_pass(self): self.assertEqual(self.cubelist[-1], self.cube2) def test_fail(self): - with self.assertRaisesRegexp(TypeError, + with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('NoneType')): self.cubelist.append(None) @@ -212,7 +212,7 @@ def test_pass(self): self.assertEqual(self.cubelist[1], self.cube2) def test_fail(self): - with self.assertRaisesRegexp(TypeError, + with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('NoneType')): self.cubelist.insert(0, None) @@ -377,7 +377,7 @@ def test_pass(self): iris.cube.CubeList([self.cube2, self.cube3, self.cube1])) def test_fail(self): - with self.assertRaisesRegexp(TypeError, + with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('NoneType')): self.cubelist[0] = None with self.assertRaisesRegexp(ValueError, From 69f892025de578be9002e919fa26b6d95d9168e2 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 25 May 2022 17:01:57 +0100 Subject: [PATCH 14/17] Replace CubeList init with new. Cube testing duck-type. --- lib/iris/cube.py | 30 +++++------ lib/iris/tests/unit/cube/test_CubeList.py | 65 ++++++++++------------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index d25d2a574e..a006f63021 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -152,16 +152,13 @@ class CubeList(list): """ - def __new__(cls, list_of_cubes=None): - """Given a :class:`list` of cubes, return a CubeList instance.""" - cube_list = list.__new__(cls, list_of_cubes) - - # Check that all items in the incoming list are cubes. - if not all([isinstance(cube, Cube) for cube in cube_list]): - raise ValueError( - "All items in list_of_cubes must be Cube " "instances." - ) - return cube_list + def __init__(self, *args, **kwargs): + """Given an iterable of cubes, return a CubeList instance.""" + # Do whatever a list does, to initialise ourself "as a list" + super().__init__(*args, **kwargs) + # Check that all items in the list are cubes. + for cube in self: + self._assert_is_cube(cube) def __str__(self): """Runs short :meth:`Cube.summary` on every cube.""" @@ -177,13 +174,16 @@ def __str__(self): def __repr__(self): """Runs repr on every cube.""" - return '[%s]' % ',\n'.join([repr(cube) for cube in self]) + return "[%s]" % ",\n".join([repr(cube) for cube in self]) @staticmethod def _assert_is_cube(obj): - if not isinstance(obj, Cube): - msg = ("Object of type '{}' does not belong in a cubelist.") - raise ValueError(msg.format(type(obj).__name__)) + if not hasattr(obj, "add_aux_coord"): + msg = ( + r"Object {obj} cannot be put in a cubelist, " + "as it is not a Cube." + ) + raise ValueError(msg) # TODO #370 Which operators need overloads? @@ -212,7 +212,7 @@ def __iadd__(self, other_cubes): """ Add a sequence of cubes to the cubelist in place. """ - super(CubeList, self).__iadd__(CubeList(other_cubes)) + return super(CubeList, self).__iadd__(CubeList(other_cubes)) def __setitem__(self, key, cube_or_sequence): """Set self[key] to cube or sequence of cubes""" diff --git a/lib/iris/tests/unit/cube/test_CubeList.py b/lib/iris/tests/unit/cube/test_CubeList.py index 9a7ad8ad9f..771b214fe4 100644 --- a/lib/iris/tests/unit/cube/test_CubeList.py +++ b/lib/iris/tests/unit/cube/test_CubeList.py @@ -10,9 +10,8 @@ import iris.tests as tests # isort:skip import collections -from unittest import mock - import copy +from unittest import mock from cf_units import Unit import numpy as np @@ -25,14 +24,15 @@ from iris.fileformats.pp import STASH import iris.tests.stock -NOT_CUBE_MSG = "Object of type '{}' does not belong in a cubelist." +NOT_CUBE_MSG = "cannot be put in a cubelist, as it is not a Cube." +NON_ITERABLE_MSG = "object is not iterable" class Test_append(tests.IrisTest): def setUp(self): self.cubelist = iris.cube.CubeList() - self.cube1 = iris.cube.Cube(1, long_name='foo') - self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cube1 = iris.cube.Cube(1, long_name="foo") + self.cube2 = iris.cube.Cube(1, long_name="bar") def test_pass(self): self.cubelist.append(self.cube1) @@ -41,8 +41,7 @@ def test_pass(self): self.assertEqual(self.cubelist[-1], self.cube2) def test_fail(self): - with self.assertRaisesRegexp(ValueError, - NOT_CUBE_MSG.format('NoneType')): + with self.assertRaisesRegex(ValueError, NOT_CUBE_MSG): self.cubelist.append(None) @@ -94,8 +93,8 @@ def test_empty(self): class Test_extend(tests.IrisTest): def setUp(self): - self.cube1 = iris.cube.Cube(1, long_name='foo') - self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cube1 = iris.cube.Cube(1, long_name="foo") + self.cube2 = iris.cube.Cube(1, long_name="bar") self.cubelist1 = iris.cube.CubeList([self.cube1]) self.cubelist2 = iris.cube.CubeList([self.cube2]) @@ -107,12 +106,11 @@ def test_pass(self): self.assertEqual(cubelist[-1], self.cube2) def test_fail(self): - with self.assertRaisesRegexp(TypeError, 'Cube is not iterable'): + with self.assertRaisesRegex(TypeError, NON_ITERABLE_MSG): self.cubelist1.extend(self.cube1) - msg = "'NoneType' object is not iterable" - with self.assertRaisesRegexp(TypeError, msg): + with self.assertRaisesRegex(TypeError, NON_ITERABLE_MSG): self.cubelist1.extend(None) - with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('int')): + with self.assertRaisesRegex(ValueError, NOT_CUBE_MSG): self.cubelist1.extend(range(3)) @@ -178,8 +176,8 @@ def test_different_orders(self): class Test_iadd(tests.IrisTest): def setUp(self): - self.cube1 = iris.cube.Cube(1, long_name='foo') - self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cube1 = iris.cube.Cube(1, long_name="foo") + self.cube2 = iris.cube.Cube(1, long_name="bar") self.cubelist1 = iris.cube.CubeList([self.cube1]) self.cubelist2 = iris.cube.CubeList([self.cube2]) @@ -191,20 +189,18 @@ def test_pass(self): self.assertEqual(cubelist[-1], self.cube2) def test_fail(self): - msg = 'Cube is not iterable' - with self.assertRaisesRegexp(TypeError, msg): + with self.assertRaisesRegex(TypeError, NON_ITERABLE_MSG): self.cubelist1 += self.cube1 - msg = "'float' object is not iterable" - with self.assertRaisesRegexp(TypeError, msg): - self.cubelist1 += 1. - with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('int')): + with self.assertRaisesRegex(TypeError, NON_ITERABLE_MSG): + self.cubelist1 += 1.0 + with self.assertRaisesRegex(ValueError, NOT_CUBE_MSG): self.cubelist1 += range(3) class Test_insert(tests.IrisTest): def setUp(self): - self.cube1 = iris.cube.Cube(1, long_name='foo') - self.cube2 = iris.cube.Cube(1, long_name='bar') + self.cube1 = iris.cube.Cube(1, long_name="foo") + self.cube2 = iris.cube.Cube(1, long_name="bar") self.cubelist = iris.cube.CubeList([self.cube1] * 3) def test_pass(self): @@ -212,8 +208,7 @@ def test_pass(self): self.assertEqual(self.cubelist[1], self.cube2) def test_fail(self): - with self.assertRaisesRegexp(ValueError, - NOT_CUBE_MSG.format('NoneType')): + with self.assertRaisesRegex(ValueError, NOT_CUBE_MSG): self.cubelist.insert(0, None) @@ -363,9 +358,9 @@ def test_combination_with_extra_triple(self): class Test_setitem(tests.IrisTest): def setUp(self): - self.cube1 = iris.cube.Cube(1, long_name='foo') - self.cube2 = iris.cube.Cube(1, long_name='bar') - self.cube3 = iris.cube.Cube(1, long_name='boo') + self.cube1 = iris.cube.Cube(1, long_name="foo") + self.cube2 = iris.cube.Cube(1, long_name="bar") + self.cube3 = iris.cube.Cube(1, long_name="boo") self.cubelist = iris.cube.CubeList([self.cube1] * 3) def test_pass(self): @@ -374,20 +369,18 @@ def test_pass(self): self.cubelist[:2] = (self.cube2, self.cube3) self.assertEqual( self.cubelist, - iris.cube.CubeList([self.cube2, self.cube3, self.cube1])) + iris.cube.CubeList([self.cube2, self.cube3, self.cube1]), + ) def test_fail(self): - with self.assertRaisesRegexp(ValueError, - NOT_CUBE_MSG.format('NoneType')): + with self.assertRaisesRegex(ValueError, NOT_CUBE_MSG): self.cubelist[0] = None - with self.assertRaisesRegexp(ValueError, - NOT_CUBE_MSG.format('NoneType')): + with self.assertRaisesRegex(ValueError, NOT_CUBE_MSG): self.cubelist[0:2] = [self.cube3, None] - msg = "can only assign an iterable" - with self.assertRaisesRegexp(TypeError, msg): + with self.assertRaisesRegex(TypeError, NON_ITERABLE_MSG): self.cubelist[:1] = 2.5 - with self.assertRaisesRegexp(TypeError, msg): + with self.assertRaisesRegex(TypeError, NON_ITERABLE_MSG): self.cubelist[:1] = self.cube1 From 7c070b89fcc4bf75f9a428760e09cf6d140a37a0 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 25 May 2022 17:04:37 +0100 Subject: [PATCH 15/17] Review change. --- lib/iris/cube.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index a006f63021..e3bacf08fc 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -225,12 +225,6 @@ def __setitem__(self, key, cube_or_sequence): super(CubeList, self).__setitem__(key, cube_or_sequence) - # __setslice__ is only required for python2.7 compatibility. - def __setslice__(self, *args): - args_list = list(args) - args_list[-1] = CubeList(args[-1]) - super(CubeList, self).__setslice__(*args_list) - def append(self, cube): """ Append a cube. From 1dd3589d8cc92b270ffb1ce406997faa5cc8a4fb Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 26 May 2022 11:02:07 +0100 Subject: [PATCH 16/17] Provide new-style whatsnew. --- .../bugfix_2018-Dec-07_only_cubes_in_cubelists.txt | 3 --- docs/src/whatsnew/latest.rst | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-) delete mode 100644 docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt diff --git a/docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt b/docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt deleted file mode 100644 index 2af5b2f537..0000000000 --- a/docs/iris/src/whatsnew/contributions_2.3.0/bugfix_2018-Dec-07_only_cubes_in_cubelists.txt +++ /dev/null @@ -1,3 +0,0 @@ -* The `append`, `extend` and `insert` methods of :class:`iris.cube.CubeList` -now perform a check to ensure that only :class:`iris.cube.Cube` instances are -added. diff --git a/docs/src/whatsnew/latest.rst b/docs/src/whatsnew/latest.rst index 7ecce0c5fb..dd571bf44d 100644 --- a/docs/src/whatsnew/latest.rst +++ b/docs/src/whatsnew/latest.rst @@ -111,6 +111,11 @@ This document explains the changes made to Iris for this release #. `@wjbenfold`_ and `@rcomer`_ (reviewer) corrected the axis on which masking is applied when an aggregator adds a trailing dimension. (:pull:`4755`) +* `@rcomer`_ and `@pp-mo`_ ensured that all methods to create or modify a +:class:`iris.cube.CubeList` check that it only contains cubes. According to +code comments, this was supposedly already the case, but there were several bugs +and loopholes. + 💣 Incompatible Changes ======================= From 7a48fd66d2c384f107b9d2976b1f11569604d400 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 26 May 2022 15:01:29 +0100 Subject: [PATCH 17/17] Fix whatsnew --- docs/src/whatsnew/latest.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/src/whatsnew/latest.rst b/docs/src/whatsnew/latest.rst index dd571bf44d..86623ed204 100644 --- a/docs/src/whatsnew/latest.rst +++ b/docs/src/whatsnew/latest.rst @@ -112,9 +112,9 @@ This document explains the changes made to Iris for this release is applied when an aggregator adds a trailing dimension. (:pull:`4755`) * `@rcomer`_ and `@pp-mo`_ ensured that all methods to create or modify a -:class:`iris.cube.CubeList` check that it only contains cubes. According to -code comments, this was supposedly already the case, but there were several bugs -and loopholes. + :class:`iris.cube.CubeList` check that it only contains cubes. According to + code comments, this was supposedly already the case, but there were several bugs + and loopholes. 💣 Incompatible Changes