Ensure cubelists only contain cubes

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.

lib/iris/cube.py

@@ -1,4 +1,4 @@
-# (C) British Crown Copyright 2010 - 2018, Met Office
+# (C) British Crown Copyright 2010 - 2019, Met Office
 #
 # This file is part of Iris.
 #
@@ -185,6 +185,32 @@ def merged(self, unique=False):
                                       self.pairs])
 
 
+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"
@@ -196,10 +222,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.')
@@ -218,8 +241,8 @@ def __str__(self):
     def __repr__(self):
         """Runs repr on every cube."""
         return '[%s]' % ',\n'.join([repr(cube) for cube in self])
-
     # TODO #370 Which operators need overloads?
+
     def __add__(self, other):
         return CubeList(list.__add__(self, other))
 
@@ -241,6 +264,56 @@ 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.
+        """
+        _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.
+            _check_iscube(cube_or_sequence)
+        else:
+            # key is a slice (or exception will come from list method).
+            _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]
+        _check_cube_sequence(cubes)
+        super(CubeList, self).__setslice__(*args)
+
+    def append(self, cube):
+        """
+        Append a cube.
+        """
+        _check_iscube(cube)
+        super(CubeList, self).append(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.
+        """
+        _check_cube_sequence(other_cubes)
+        super(CubeList, self).extend(other_cubes)
+
+    def insert(self, index, cube):
+        """
+        Insert a cube before index.
+        """
+        _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."""
         doc = Document()

lib/iris/tests/unit/cube/test_CubeList.py

@@ -1,4 +1,4 @@
-# (C) British Crown Copyright 2014 - 2018, Met Office
+# (C) British Crown Copyright 2014 - 2019, Met Office
 #
 # This file is part of Iris.
 #
@@ -24,6 +24,8 @@
 import iris.tests as tests
 import iris.tests.stock
 
+import copy
+
 from cf_units import Unit
 import numpy as np
 
@@ -34,6 +36,26 @@
 from iris.fileformats.pp import STASH
 from iris.tests import mock
 
+NOT_CUBE_MSG = "bject of type '{}'"
+
+
+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):
+        with self.assertRaisesRegexp(TypeError,
+                                     NOT_CUBE_MSG.format('NoneType')):
+            self.cubelist.append(None)
+
 
 class Test_concatenate_cube(tests.IrisTest):
     def setUp(self):
@@ -64,6 +86,30 @@ 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 = copy.copy(self.cubelist1)
+        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):
+        with self.assertRaisesRegexp(TypeError, 'Cube is not iterable'):
+            self.cubelist1.extend(self.cube1)
+        msg = "'NoneType' object is not iterable"
+        with self.assertRaisesRegexp(TypeError, msg):
+            self.cubelist1.extend(None)
+        with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('int')):
+            self.cubelist1.extend(range(3))
+
+
 class Test_extract_overlapping(tests.IrisTest):
     def setUp(self):
         shape = (6, 14, 19)
@@ -118,6 +164,47 @@ 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 = copy.copy(self.cubelist1)
+        cubelist += self.cubelist2
+        self.assertEqual(cubelist, self.cubelist1 + self.cubelist2)
+        cubelist += [self.cube2]
+        self.assertEqual(cubelist[-1], self.cube2)
+
+    def test_fail(self):
+        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.
+        with self.assertRaisesRegexp(ValueError, NOT_CUBE_MSG.format('int')):
+            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.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):
+        with self.assertRaisesRegexp(TypeError,
+                                     NOT_CUBE_MSG.format('NoneType')):
+            self.cubelist.insert(0, None)
+
+
 class Test_merge_cube(tests.IrisTest):
     def setUp(self):
         self.cube1 = Cube([1, 2, 3], "air_temperature", units="K")
@@ -241,6 +328,36 @@ 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.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):
+        with self.assertRaisesRegexp(TypeError,
+                                     NOT_CUBE_MSG.format('NoneType')):
+            self.cubelist[0] = None
+        with self.assertRaisesRegexp(ValueError,
+                                     NOT_CUBE_MSG.format('NoneType')):
+            self.cubelist[0:2] = [self.cube3, None]
+
+        msg = "can only assign an iterable"
+        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):
     def setUp(self):
         self.cubes = CubeList([Cube(np.arange(3)),