From 74e8290015a2ebedd4c3d2c7cf16214c2a871d1d Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Wed, 3 May 2017 10:50:07 +0100
Subject: [PATCH 1/7] Fix concatenate fill value.

---
 lib/iris/_concatenate.py                      | 21 ++--
 .../concatenate/concat_masked_2x2d.cml        |  2 +-
 .../concatenate/concat_masked_2y2d.cml        |  2 +-
 lib/iris/tests/test_concatenate.py            | 97 ++++++++-----------
 4 files changed, 49 insertions(+), 73 deletions(-)

diff --git a/lib/iris/_concatenate.py b/lib/iris/_concatenate.py
index 8b41936e98..ef1b93274d 100644
--- a/lib/iris/_concatenate.py
+++ b/lib/iris/_concatenate.py
@@ -416,7 +416,6 @@ def match(self, other, error_on_mismatch):
             - scalar coords
             - attributes
             - dtype
-            - fill_value
 
         Args:
 
@@ -465,14 +464,6 @@ def match(self, other, error_on_mismatch):
             msgs.append(msg_template.format('Data types', '',
                                             self.data_type, other.data_type))
 
-        # Check fill value.
-        if self.fill_value != other.fill_value:
-            # Allow fill-value promotion if either fill-value is None.
-            if self.fill_value is not None and other.fill_value is not None:
-                msgs.append(msg_template.format('Fill values', '',
-                                                self.fill_value,
-                                                other.fill_value))
-
         # Check _cell_measures_and_dims
         if self.cell_measures_and_dims != other.cell_measures_and_dims:
             msgs.append(msg_template.format('CellMeasures', '',
@@ -727,10 +718,14 @@ def register(self, cube, axis=None, error_on_mismatch=False,
 
         # Check for compatible coordinate signatures.
         if match:
-            if self._cube_signature.fill_value is None and \
-                    cube_signature.fill_value is not None:
-                # Perform proto-cube fill-value promotion.
-                self._cube_signature.fill_value = cube_signature.fill_value
+            fill_value = self._cube_signature.fill_value
+            # Determine whether the fill value requires to be
+            # demoted to the default value.
+            if fill_value is not None:
+                if cube_signature.fill_value is None or \
+                        cube_signature.fill_value != fill_value:
+                    # Demote the fill value to the default.
+                    self._cube_signature.fill_value = None
             coord_signature = _CoordSignature(cube_signature)
             candidate_axis = self._coord_signature.candidate_axis(
                 coord_signature)
diff --git a/lib/iris/tests/results/concatenate/concat_masked_2x2d.cml b/lib/iris/tests/results/concatenate/concat_masked_2x2d.cml
index 92d00b09a5..d1fa542249 100644
--- a/lib/iris/tests/results/concatenate/concat_masked_2x2d.cml
+++ b/lib/iris/tests/results/concatenate/concat_masked_2x2d.cml
@@ -1,6 +1,6 @@
 <?xml version="1.0" ?>
 <cubes xmlns="urn:x-iris:cubeml-0.2">
-  <cube core-dtype="float32" dtype="float32" fill_value="1234.0" units="unknown">
+  <cube core-dtype="float32" dtype="float32" units="unknown">
     <coords>
       <coord datadims="[1]">
         <dimCoord bounds="[[-0.5, 0.5],
diff --git a/lib/iris/tests/results/concatenate/concat_masked_2y2d.cml b/lib/iris/tests/results/concatenate/concat_masked_2y2d.cml
index c621ece9f0..10f67f30e2 100644
--- a/lib/iris/tests/results/concatenate/concat_masked_2y2d.cml
+++ b/lib/iris/tests/results/concatenate/concat_masked_2y2d.cml
@@ -1,6 +1,6 @@
 <?xml version="1.0" ?>
 <cubes xmlns="urn:x-iris:cubeml-0.2">
-  <cube core-dtype="float32" dtype="float32" fill_value="1234.0" units="unknown">
+  <cube core-dtype="float32" dtype="float32" units="unknown">
     <coords>
       <coord datadims="[1]">
         <dimCoord bounds="[[-0.5, 0.5],
diff --git a/lib/iris/tests/test_concatenate.py b/lib/iris/tests/test_concatenate.py
index aaf203f0bf..85e5181db6 100644
--- a/lib/iris/tests/test_concatenate.py
+++ b/lib/iris/tests/test_concatenate.py
@@ -335,47 +335,23 @@ def test_order_difference(self):
         result = concatenate(cubes)
         self.assertEqual(len(result), 2)
 
-    def test_masked_fill_value_difference(self):
-        cubes = []
-        y = (0, 2)
-        cube = _make_cube((0, 2), y, 1, mask=True)
-        cube.fill_value = 10
-        cubes.append(cube)
-        cube = _make_cube((2, 4), y, 1, mask=True)
-        cube.fill_value = 20
-        cubes.append(cube)
-        result = concatenate(cubes)
-        self.assertEqual(len(result), 2)
-
-    def test_masked_fill_value_difference__exception(self):
-        cubes = iris.cube.CubeList()
-        y = (0, 2)
-        cube = _make_cube((0, 2), y, 1, mask=True)
-        cube.fill_value = 10
-        cubes.append(cube)
-        cube = _make_cube((2, 4), y, 1, mask=True)
-        cube.fill_value = 20
-        cubes.append(cube)
-        emsg = 'Fill values differ'
-        with self.assertRaisesRegexp(iris.exceptions.ConcatenateError, emsg):
-            cubes.concatenate_cube()
-
 
 class Test2D(tests.IrisTest):
-    def test_masked_and_unmasked(self):
+    def test_masked_and_unmasked__fill_value_equal(self):
         cubes = []
         y = (0, 2)
         fill_value = 1234
         cube = _make_cube((0, 2), y, 1, mask=True,
                           fill_value=fill_value)
         cubes.append(cube)
-        cubes.append(_make_cube((2, 4), y, 2))
+        cubes.append(_make_cube((2, 4), y, 2,
+                                fill_value=fill_value))
         result = concatenate(cubes)
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].fill_value, fill_value)
         self.assertEqual(result[0].data.fill_value, fill_value)
 
-    def test_masked_and_unmasked_int16(self):
+    def test_masked_and_unmasked_int16__fill_value_equal(self):
         cubes = []
         y = (0, 2)
         dtype = np.dtype('int16')
@@ -383,48 +359,54 @@ def test_masked_and_unmasked_int16(self):
         cube = _make_cube((0, 2), y, 1, dtype=dtype, mask=True,
                           fill_value=fill_value)
         cubes.append(cube)
-        cubes.append(_make_cube((2, 4), y, 2, dtype=dtype))
+        cubes.append(_make_cube((2, 4), y, 2, dtype=dtype,
+                                fill_value=fill_value))
         result = concatenate(cubes)
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].fill_value, fill_value)
         self.assertEqual(result[0].data.fill_value, fill_value)
 
-    def test_unmasked_and_masked(self):
+    def test_unmasked_and_masked__default_fill_value_from_None(self):
         cubes = []
         y = (0, 2)
         cubes.append(_make_cube((0, 2), y, 1))
-        fill_value = 1234
         cube = _make_cube((2, 4), y, 2, mask=True,
-                          fill_value=fill_value)
+                          fill_value=1234)
         cubes.append(cube)
         result = concatenate(cubes)
         self.assertEqual(len(result), 1)
-        self.assertEqual(result[0].fill_value, fill_value)
-        self.assertEqual(result[0].data.fill_value, fill_value)
+        self.assertIsNone(result[0].fill_value)
+        np_fill_value = ma.masked_array(0, dtype=result[0].dtype).fill_value
+        self.assertEqual(result[0].data.fill_value, np_fill_value)
 
-    def test_unmasked_and_masked_int16(self):
+    def test_unmasked_and_masked_int16__default_fill_value_from_diff(self):
         cubes = []
         y = (0, 2)
         dtype = np.dtype('int16')
-        cubes.append(_make_cube((0, 2), y, 1, dtype=dtype))
-        fill_value = 1234
+        cubes.append(_make_cube((0, 2), y, 1, dtype=dtype, fill_value=1234))
         cube = _make_cube((2, 4), y, 2, dtype=dtype, mask=True,
-                          fill_value=fill_value)
+                          fill_value=4321)
         cubes.append(cube)
         result = concatenate(cubes)
         self.assertEqual(len(result), 1)
-        self.assertEqual(result[0].fill_value, fill_value)
-        self.assertEqual(result[0].data.fill_value, fill_value)
+        self.assertIsNone(result[0].fill_value)
+        np_fill_value = ma.masked_array(0, dtype=result[0].dtype).fill_value
+        self.assertEqual(result[0].data.fill_value, np_fill_value)
 
-    def test_masked_with_data_fill_value_difference(self):
+    def test_masked_with_data_fill_value_difference_on_arrays(self):
         cubes = []
         y = (0, 2)
         fill_value = 1234
         cube = _make_cube((0, 2), y, 1, mask=True,
                           fill_value=fill_value)
+        # Setting the actual fill value on the array,
+        # which should make no difference.
         cube._data_manager._real_array.fill_value = 10
         cubes.append(cube)
-        cube = _make_cube((2, 4), y, 1, mask=True)
+        cube = _make_cube((2, 4), y, 1, mask=True,
+                          fill_value=fill_value)
+        # Setting the actual fill value on the array,
+        # which should make no difference.
         cube._data_manager._real_array.fill_value = 20
         cubes.append(cube)
         result = concatenate(cubes)
@@ -432,16 +414,14 @@ def test_masked_with_data_fill_value_difference(self):
         self.assertEqual(result[0].fill_value, fill_value)
         self.assertEqual(result[0].data.fill_value, fill_value)
 
-    def test_concat_masked_2x2d(self):
+    def test_concat_masked_2x2d__default_fill_value_from_None(self):
         cubes = []
         y = (0, 2)
         mask = [(0, 1), (0, 1)]
-        cube = _make_cube((0, 2), y, 1, mask=mask)
+        cube = _make_cube((0, 2), y, 1, mask=mask, fill_value=1234)
         cubes.append(cube)
         mask = [(0, 1), (1, 0)]
-        fill_value = 1234
-        cube = _make_cube((2, 4), y, 2, mask=mask,
-                          fill_value=fill_value)
+        cube = _make_cube((2, 4), y, 2, mask=mask)
         cubes.append(cube)
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_masked_2x2d.cml'))
@@ -450,19 +430,18 @@ def test_concat_masked_2x2d(self):
         mask = np.array([[True, False, False, True],
                          [False, True, True, False]], dtype=np.bool)
         self.assertArrayEqual(result[0].data.mask, mask)
-        self.assertEqual(result[0].fill_value, fill_value)
-        self.assertEqual(result[0].data.fill_value, fill_value)
+        self.assertIsNone(result[0].fill_value)
+        np_fill_value = ma.masked_array(0, dtype=result[0].dtype).fill_value
+        self.assertEqual(result[0].data.fill_value, np_fill_value)
 
-    def test_concat_masked_2y2d(self):
+    def test_concat_masked_2y2d__default_fill_value_from_diff(self):
         cubes = []
         x = (0, 2)
         mask = [(0, 1), (0, 1)]
-        fill_value = 1234
-        cube = _make_cube(x, (0, 2), 1, mask=mask,
-                          fill_value=fill_value)
+        cube = _make_cube(x, (0, 2), 1, mask=mask, fill_value=1234)
         cubes.append(cube)
         mask = [(0, 1), (1, 0)]
-        cube = _make_cube(x, (2, 4), 2, mask=mask)
+        cube = _make_cube(x, (2, 4), 2, mask=mask, fill_value=4321)
         cubes.append(cube)
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_masked_2y2d.cml'))
@@ -473,8 +452,9 @@ def test_concat_masked_2y2d(self):
                          [False, True],
                          [True, False]], dtype=np.bool)
         self.assertArrayEqual(result[0].data.mask, mask)
-        self.assertEqual(result[0].fill_value, fill_value)
-        self.assertEqual(result[0].data.fill_value, fill_value)
+        self.assertIsNone(result[0].fill_value)
+        np_fill_value = ma.masked_array(0, dtype=result[0].dtype).fill_value
+        self.assertEqual(result[0].data.fill_value, np_fill_value)
 
     def test_concat_masked_2y2d_int16(self):
         cubes = []
@@ -512,7 +492,7 @@ def test_concat_masked_2y2d_int16_with_concrete_and_lazy(self):
         cubes.append(cube)
         mask = [(0, 1), (1, 0)]
         cube = _make_cube(x, (2, 4), 2, dtype=dtype, mask=mask)
-        cube.replace(cube.lazy_data(), dtype=dtype)
+        cube.replace(cube.lazy_data(), dtype=dtype, fill_value=fill_value)
         cubes.append(cube)
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_masked_2y2d_int16.cml'))
@@ -536,7 +516,8 @@ def test_concat_masked_2y2d_int16_with_lazy_and_concrete(self):
         cube.replace(cube.lazy_data(), dtype=dtype, fill_value=fill_value)
         cubes.append(cube)
         mask = [(0, 1), (0, 1)]
-        cube = _make_cube(x, (0, 2), 1, dtype=dtype, mask=mask)
+        cube = _make_cube(x, (0, 2), 1, dtype=dtype, mask=mask,
+                          fill_value=fill_value)
         cubes.append(cube)
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_masked_2y2d_int16.cml'))

From 703a49a29e8398bf2ffc78ce74b16bedf069ad71 Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Wed, 3 May 2017 12:02:19 +0100
Subject: [PATCH 2/7] Fix merge fill value.

---
 lib/iris/_merge.py           | 37 ++++++++--------
 lib/iris/tests/test_merge.py | 83 +++++++++++++++++++++++-------------
 2 files changed, 72 insertions(+), 48 deletions(-)

diff --git a/lib/iris/_merge.py b/lib/iris/_merge.py
index 9bb40c777e..35121dcfae 100644
--- a/lib/iris/_merge.py
+++ b/lib/iris/_merge.py
@@ -407,11 +407,6 @@ def match(self, other, error_on_mismatch):
         if self.data_type != other.data_type:
             msg = 'cube data dtype differs: {} != {}'
             msgs.append(msg.format(self.data_type, other.data_type))
-        if self.fill_value != other.fill_value:
-            # Allow fill-value promotion if either fill-value is None.
-            if self.fill_value is not None and other.fill_value is not None:
-                msg = 'cube fill value differs: {} != {}'
-                msgs.append(msg.format(self.fill_value, other.fill_value))
         if (self.cell_measures_and_dims != other.cell_measures_and_dims):
             msgs.append('cube.cell_measures differ')
 
@@ -1285,15 +1280,19 @@ def register(self, cube, error_on_mismatch=False):
             this :class:`ProtoCube`.
 
         """
-        signature = self._cube_signature
+        cube_signature = self._cube_signature
         other = self._build_signature(cube)
-        match = signature.match(other, error_on_mismatch)
+        match = cube_signature.match(other, error_on_mismatch)
         if match:
-            if signature.fill_value is None and other.fill_value is not None:
-                # Perform proto-cube fill-value promotion.
-                fv = other.fill_value
-                self._cube_signature = self._build_signature(self._source,
-                                                             fill_value=fv)
+            # Determine whether the fill value requires to be demoted
+            # to the default value.
+            if cube_signature.fill_value is not None:
+                if other.fill_value is None or \
+                        cube_signature.fill_value != other.fill_value:
+                    # Demote the fill value to the default.
+                    signature = self._build_signature(self._source,
+                                                      default_fill_value=True)
+                    self._cube_signature = signature
             coord_payload = self._extract_coord_payload(cube)
             match = coord_payload.match_signature(self._coord_signature,
                                                   error_on_mismatch)
@@ -1614,7 +1613,7 @@ def _build_coordinates(self):
                               self._vector_aux_coords_dims):
             aux_coords_and_dims.append(_CoordAndDims(item.coord, dims))
 
-    def _build_signature(self, cube, fill_value=None):
+    def _build_signature(self, cube, default_fill_value=False):
         """
         Generate the signature that defines this cube.
 
@@ -1625,16 +1624,18 @@ def _build_signature(self, cube, fill_value=None):
 
         Kwargs:
 
-        * fill_value:
-            Promoted fill-value to use instead of the source cube
-            fill-value.
+        * default_fill_value:
+            Override the cube fill value with the default fill value of None.
+            Default is False i.e. use the provided cube.fill_value when
+            constructing the cube signature.
 
         Returns:
             The cube signature.
 
         """
-        if fill_value is None:
-            fill_value = cube.fill_value
+        fill_value = cube.fill_value
+        if default_fill_value:
+            fill_value = None
         return _CubeSignature(cube.metadata, cube.shape, cube.dtype,
                               fill_value, cube._cell_measures_and_dims)
 
diff --git a/lib/iris/tests/test_merge.py b/lib/iris/tests/test_merge.py
index 17e7c69238..a6efcc6f1e 100644
--- a/lib/iris/tests/test_merge.py
+++ b/lib/iris/tests/test_merge.py
@@ -145,81 +145,104 @@ def _make_cube(self, data, dtype=np.dtype('int32'), fill_value=None,
         cube.add_aux_coord(height)
         return cube
 
+    @staticmethod
+    def _expected_fill(fill0, fill1):
+        result = None
+        if fill0 == fill1:
+            result = fill0
+        return result
+
+    def _check_fill_value(self, result, fill0, fill1):
+        fill_value = self._expected_fill(fill0, fill1)
+        if fill_value is None:
+            fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertIsNone(result.fill_value)
+            data = result.data
+            if ma.isMaskedArray(data):
+                self.assertEqual(data.fill_value, fill_value)
+        else:
+            self.assertEqual(result.fill_value, fill_value)
+            data = result.data
+            if ma.isMaskedArray(data):
+                self.assertEqual(data.fill_value, fill_value)
+
     def setUp(self):
         self.dtype = np.dtype('int32')
-        self.fill_value = 1234
-        self.lazy_combos = itertools.product([False, True],
-                                             [False, True])
+        fill_value = 1234
+        lazy_combos = itertools.product([False, True],
+                                        [False, True])
+        fill_combos = itertools.product([None, fill_value],
+                                        [fill_value, None])
+        self.combos = itertools.product(lazy_combos, fill_combos)
 
     def test__ndarray_ndarray(self):
-        for lazy0, lazy1 in self.lazy_combos:
+        for (lazy0, lazy1), (fill0, fill1) in self.combos:
             cubes = iris.cube.CubeList()
-            cubes.append(self._make_cube(0, dtype=self.dtype, lazy=lazy0))
-            cubes.append(self._make_cube(1, dtype=self.dtype, lazy=lazy1))
+            cubes.append(self._make_cube(0, dtype=self.dtype, lazy=lazy0,
+                                         fill_value=fill0))
+            cubes.append(self._make_cube(1, dtype=self.dtype, lazy=lazy1,
+                                         fill_value=fill1))
             result = cubes.merge_cube()
             expected = self._make_data([0, 1], dtype=self.dtype)
             self.assertArrayEqual(result.data, expected)
             self.assertEqual(result.dtype, self.dtype)
+            self._check_fill_value(result, fill0, fill1)
 
     def test__masked_masked(self):
-        for lazy0, lazy1 in self.lazy_combos:
+        for (lazy0, lazy1), (fill0, fill1) in self.combos:
             cubes = iris.cube.CubeList()
             mask = [(0,), (0,)]
             cubes.append(self._make_cube(0, mask=mask, lazy=lazy0,
                                          dtype=self.dtype,
-                                         fill_value=self.fill_value))
+                                         fill_value=fill0))
             mask = [(1,), (1,)]
             cubes.append(self._make_cube(1, mask=mask, lazy=lazy1,
                                          dtype=self.dtype,
-                                         fill_value=self.fill_value))
+                                         fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(0, 1), (0, 1), (0, 1)]
+            fill_value = self._expected_fill(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
-                                       fill_value=self.fill_value)
+                                       fill_value=fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
-            self.assertEqual(result.fill_value, self.fill_value)
             self.assertEqual(result.dtype, self.dtype)
+            self._check_fill_value(result, fill0, fill1)
 
     def test__ndarray_masked(self):
-        for lazy0, lazy1 in self.lazy_combos:
+        for (lazy0, lazy1), (fill0, fill1) in self.combos:
             cubes = iris.cube.CubeList()
-            cubes.append(self._make_cube(0, lazy=lazy0, dtype=self.dtype))
+            cubes.append(self._make_cube(0, lazy=lazy0, dtype=self.dtype,
+                                         fill_value=fill0))
             mask = [(0, 1), (0, 1)]
             cubes.append(self._make_cube(1, mask=mask, lazy=lazy1,
                                          dtype=self.dtype,
-                                         fill_value=self.fill_value))
+                                         fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(1, 1), (0, 1), (0, 1)]
+            fill_value = self._expected_fill(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
-                                       fill_value=self.fill_value)
+                                       fill_value=fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
-            self.assertEqual(result.fill_value, self.fill_value)
             self.assertEqual(result.dtype, self.dtype)
+            self._check_fill_value(result, fill0, fill1)
 
     def test__masked_ndarray(self):
-        for lazy0, lazy1 in self.lazy_combos:
+        for (lazy0, lazy1), (fill0, fill1) in self.combos:
             cubes = iris.cube.CubeList()
             mask = [(0, 1), (0, 1)]
             cubes.append(self._make_cube(0, mask=mask, lazy=lazy0,
                                          dtype=self.dtype,
-                                         fill_value=self.fill_value))
-            cubes.append(self._make_cube(1, lazy=lazy1, dtype=self.dtype))
+                                         fill_value=fill0))
+            cubes.append(self._make_cube(1, lazy=lazy1, dtype=self.dtype,
+                                         fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(0, 0), (0, 1), (0, 1)]
+            fill_value = self._expected_fill(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
-                                       fill_value=self.fill_value)
+                                       fill_value=fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
-            self.assertEqual(result.fill_value, self.fill_value)
             self.assertEqual(result.dtype, self.dtype)
-
-    def test_merge_fail_on_different_fill_values(self):
-        pairs = [(0, 1234), (1, 4321)]
-        cubes = iris.cube.CubeList()
-        for data, fill_value in pairs:
-            cubes.append(self._make_cube(data, fill_value=fill_value))
-        emsg = 'cube fill value differs'
-        with self.assertRaisesRegexp(iris.exceptions.MergeError, emsg):
-            cubes.merge_cube()
+            self._check_fill_value(result, fill0, fill1)
 
 
 @tests.skip_data

From f6a46aa0db77ad032d069a0f5b04e288753e6392 Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Wed, 3 May 2017 12:41:45 +0100
Subject: [PATCH 3/7] Added merge invariant tests.

---
 lib/iris/tests/test_merge.py | 39 ++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/lib/iris/tests/test_merge.py b/lib/iris/tests/test_merge.py
index a6efcc6f1e..60bc59c6f0 100644
--- a/lib/iris/tests/test_merge.py
+++ b/lib/iris/tests/test_merge.py
@@ -244,6 +244,45 @@ def test__masked_ndarray(self):
             self.assertEqual(result.dtype, self.dtype)
             self._check_fill_value(result, fill0, fill1)
 
+    def test_fill_value_invariant_to_order__same_non_None(self):
+        fill_value = 1234
+        cubes = [self._make_cube(i, mask=True,
+                                 fill_value=fill_value) for i in range(3)]
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).merge_cube()
+            self.assertEqual(result.fill_value, fill_value)
+            self.assertEqual(result.data.fill_value, fill_value)
+
+    def test_fill_value_invariant_to_order__all_None(self):
+        cubes = [self._make_cube(i, mask=True,
+                                 fill_value=None) for i in range(3)]
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).merge_cube()
+            self.assertIsNone(result.fill_value)
+            np_fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertEqual(result.data.fill_value, np_fill_value)
+
+    def test_fill_value_invariant_to_order__different_non_None(self):
+        cubes = [self._make_cube(0, mask=True, fill_value=1234)]
+        cubes.append(self._make_cube(1, mask=True, fill_value=2341))
+        cubes.append(self._make_cube(2, mask=True, fill_value=3412))
+        cubes.append(self._make_cube(3, mask=True, fill_value=4123))
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).merge_cube()
+            self.assertIsNone(result.fill_value)
+            np_fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertEqual(result.data.fill_value, np_fill_value)
+
+    def test_fill_value_invariant_to_order__mixed(self):
+        cubes = [self._make_cube(0, mask=True, fill_value=None)]
+        cubes.append(self._make_cube(1, mask=True, fill_value=1234))
+        cubes.append(self._make_cube(2, mask=True, fill_value=4321))
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).merge_cube()
+            self.assertIsNone(result.fill_value)
+            np_fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertEqual(result.data.fill_value, np_fill_value)
+
 
 @tests.skip_data
 class TestDataMerge(tests.IrisTest):

From fb0a3a51b308d7966c408b6ec2e498667cff29c9 Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Wed, 3 May 2017 12:58:31 +0100
Subject: [PATCH 4/7] Added concatenate invariant tests.

---
 lib/iris/tests/test_concatenate.py | 44 ++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/lib/iris/tests/test_concatenate.py b/lib/iris/tests/test_concatenate.py
index 85e5181db6..24c7a49675 100644
--- a/lib/iris/tests/test_concatenate.py
+++ b/lib/iris/tests/test_concatenate.py
@@ -26,6 +26,7 @@
 # before importing anything else.
 import iris.tests as tests
 
+import itertools
 import numpy as np
 import numpy.ma as ma
 
@@ -729,6 +730,49 @@ def test_concat_2x2d_aux_xy_bounds(self):
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].shape, (2, 4))
 
+    def test_fill_value_invariant_to_order__same_non_None(self):
+        y = (0, 2)
+        fill_value = 1234
+        cubes = [_make_cube((i*2, i*2+2), y, i, mask=True,
+                            fill_value=fill_value) for i in range(3)]
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).concatenate_cube()
+            self.assertEqual(result.fill_value, fill_value)
+            self.assertEqual(result.data.fill_value, fill_value)
+
+    def test_fill_value_invariant_to_order__all_None(self):
+        y = (0, 2)
+        cubes = [_make_cube((i*2, i*2+2), y, i, mask=True,
+                            fill_value=None) for i in range(3)]
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).concatenate_cube()
+            self.assertIsNone(result.fill_value)
+            np_fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertEqual(result.data.fill_value, np_fill_value)
+
+    def test_fill_value_invariant_to_order__different_non_None(self):
+        y = (0, 2)
+        cubes = [_make_cube((0, 2), y, 0, mask=True, fill_value=1234)]
+        cubes.append(_make_cube((2, 4), y, 1, mask=True, fill_value=2341))
+        cubes.append(_make_cube((4, 6), y, 2, mask=True, fill_value=3412))
+        cubes.append(_make_cube((6, 8), y, 3, mask=True, fill_value=4123))
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).concatenate_cube()
+            self.assertIsNone(result.fill_value)
+            np_fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertEqual(result.data.fill_value, np_fill_value)
+
+    def test_fill_value_invariant_to_order__mixed(self):
+        y = (0, 2)
+        cubes = [_make_cube((0, 2), y, 0, mask=True, fill_value=None)]
+        cubes.append(_make_cube((2, 4), y, 1, mask=True, fill_value=1234))
+        cubes.append(_make_cube((4, 6), y, 2, mask=True, fill_value=4321))
+        for combo in itertools.permutations(cubes):
+            result = iris.cube.CubeList(combo).concatenate_cube()
+            self.assertIsNone(result.fill_value)
+            np_fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+            self.assertEqual(result.data.fill_value, np_fill_value)
+
 
 class TestMulti2D(tests.IrisTest):
     def test_concat_4x2d_aux_xy(self):

From e222205d9e542ebcb216d1129540c70b0d5ba9ac Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Thu, 4 May 2017 11:57:16 +0100
Subject: [PATCH 5/7] Review actions.

---
 .../tests/results/concatenate/concat_2y2d.cml |  2 +-
 lib/iris/tests/test_concatenate.py            | 46 +++++++++++++++----
 lib/iris/tests/test_merge.py                  | 10 ++--
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/lib/iris/tests/results/concatenate/concat_2y2d.cml b/lib/iris/tests/results/concatenate/concat_2y2d.cml
index 9c11ea11ba..533cadc621 100644
--- a/lib/iris/tests/results/concatenate/concat_2y2d.cml
+++ b/lib/iris/tests/results/concatenate/concat_2y2d.cml
@@ -1,6 +1,6 @@
 <?xml version="1.0" ?>
 <cubes xmlns="urn:x-iris:cubeml-0.2">
-  <cube core-dtype="float32" dtype="float32" units="unknown">
+  <cube core-dtype="float32" dtype="float32" fill_value="1234.0" units="unknown">
     <coords>
       <coord datadims="[1]">
         <dimCoord bounds="[[-0.5, 0.5],
diff --git a/lib/iris/tests/test_concatenate.py b/lib/iris/tests/test_concatenate.py
index 24c7a49675..ee03370509 100644
--- a/lib/iris/tests/test_concatenate.py
+++ b/lib/iris/tests/test_concatenate.py
@@ -93,6 +93,7 @@ def _make_cube(x, y, data, aux=None, offset=0, scalar=None,
     else:
         cube_data = np.empty(shape, dtype=dtype)
         cube_data[:] = data
+
     cube = iris.cube.Cube(cube_data, fill_value=fill_value)
     coord = DimCoord(y_range, long_name='y')
     coord.guess_bounds()
@@ -338,6 +339,30 @@ def test_order_difference(self):
 
 
 class Test2D(tests.IrisTest):
+    def test_masked_and_unmasked__default_fill_value(self):
+        cubes = []
+        y = (0, 2)
+        cube = _make_cube((0, 2), y, 1, mask=True)
+        cubes.append(cube)
+        cubes.append(_make_cube((2, 4), y, 2))
+        result = concatenate(cubes)
+        self.assertEqual(len(result), 1)
+        self.assertIsNone(result[0].fill_value)
+        np_fill_value = ma.masked_array(0, dtype=result[0].dtype).fill_value
+        self.assertEqual(result[0].data.fill_value, np_fill_value)
+
+    def test_unmasked_and_masked__default_fill_value(self):
+        cubes = []
+        y = (0, 2)
+        cubes.append(_make_cube((0, 2), y, 1))
+        cube = _make_cube((2, 4), y, 2, mask=True)
+        cubes.append(cube)
+        result = concatenate(cubes)
+        self.assertEqual(len(result), 1)
+        self.assertIsNone(result[0].fill_value)
+        np_fill_value = ma.masked_array(0, dtype=result[0].dtype).fill_value
+        self.assertEqual(result[0].data.fill_value, np_fill_value)
+
     def test_masked_and_unmasked__fill_value_equal(self):
         cubes = []
         y = (0, 2)
@@ -532,7 +557,7 @@ def test_concat_masked_2y2d_int16_with_lazy_and_concrete(self):
         self.assertEqual(result[0].fill_value, fill_value)
         self.assertEqual(result[0].data.fill_value, fill_value)
 
-    def test_concat_2x2d(self):
+    def test_concat_2x2d__default_fill_value(self):
         cubes = []
         y = (0, 2)
         cubes.append(_make_cube((0, 4), y, 1))
@@ -541,36 +566,41 @@ def test_concat_2x2d(self):
         self.assertCML(result, ('concatenate', 'concat_2x2d.cml'))
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].shape, (2, 6))
+        self.assertIsNone(result[0].fill_value)
 
-    def test_concat_2y2d(self):
+    def test_concat_2y2d__fill_value_equal(self):
         cubes = []
         x = (0, 2)
-        cubes.append(_make_cube(x, (0, 4), 1))
-        cubes.append(_make_cube(x, (4, 6), 2))
+        fill_value = 1234
+        cubes.append(_make_cube(x, (0, 4), 1, fill_value=fill_value))
+        cubes.append(_make_cube(x, (4, 6), 2, fill_value=fill_value))
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_2y2d.cml'))
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].shape, (6, 2))
+        self.assertEqual(result[0].fill_value, fill_value)
 
-    def test_concat_2x2d_aux_x(self):
+    def test_concat_2x2d_aux_x__default_fill_value_from_diff(self):
         cubes = []
         y = (0, 2)
-        cubes.append(_make_cube((0, 4), y, 1, aux='x'))
+        cubes.append(_make_cube((0, 4), y, 1, aux='x', fill_value=1234))
         cubes.append(_make_cube((4, 6), y, 2, aux='x'))
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_2x2d_aux_x.cml'))
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].shape, (2, 6))
+        self.assertIsNone(result[0].fill_value)
 
-    def test_concat_2y2d_aux_x(self):
+    def test_concat_2y2d_aux_x__default_fill_value_from_diff(self):
         cubes = []
         x = (0, 2)
         cubes.append(_make_cube(x, (0, 4), 1, aux='x'))
-        cubes.append(_make_cube(x, (4, 6), 2, aux='x'))
+        cubes.append(_make_cube(x, (4, 6), 2, aux='x', fill_value=1234))
         result = concatenate(cubes)
         self.assertCML(result, ('concatenate', 'concat_2y2d_aux_x.cml'))
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].shape, (6, 2))
+        self.assertIsNone(result[0].fill_value)
 
     def test_concat_2x2d_aux_y(self):
         cubes = []
diff --git a/lib/iris/tests/test_merge.py b/lib/iris/tests/test_merge.py
index 60bc59c6f0..0a2ab12799 100644
--- a/lib/iris/tests/test_merge.py
+++ b/lib/iris/tests/test_merge.py
@@ -146,14 +146,14 @@ def _make_cube(self, data, dtype=np.dtype('int32'), fill_value=None,
         return cube
 
     @staticmethod
-    def _expected_fill(fill0, fill1):
+    def _expected_fill_value(fill0, fill1):
         result = None
         if fill0 == fill1:
             result = fill0
         return result
 
     def _check_fill_value(self, result, fill0, fill1):
-        fill_value = self._expected_fill(fill0, fill1)
+        fill_value = self._expected_fill_value(fill0, fill1)
         if fill_value is None:
             fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
             self.assertIsNone(result.fill_value)
@@ -201,7 +201,7 @@ def test__masked_masked(self):
                                          fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(0, 1), (0, 1), (0, 1)]
-            fill_value = self._expected_fill(fill0, fill1)
+            fill_value = self._expected_fill_value(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
                                        fill_value=fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
@@ -219,7 +219,7 @@ def test__ndarray_masked(self):
                                          fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(1, 1), (0, 1), (0, 1)]
-            fill_value = self._expected_fill(fill0, fill1)
+            fill_value = self._expected_fill_value(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
                                        fill_value=fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
@@ -237,7 +237,7 @@ def test__masked_ndarray(self):
                                          fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(0, 0), (0, 1), (0, 1)]
-            fill_value = self._expected_fill(fill0, fill1)
+            fill_value = self._expected_fill_value(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
                                        fill_value=fill_value)
             self.assertMaskedArrayEqual(result.data, expected)

From d44458d1125f3a2f95092e46a511c2b7a32ad05c Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Thu, 4 May 2017 13:47:57 +0100
Subject: [PATCH 6/7] Simplify merge/concatenate match condition.

---
 lib/iris/_concatenate.py | 3 +--
 lib/iris/_merge.py       | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/iris/_concatenate.py b/lib/iris/_concatenate.py
index ef1b93274d..0029541617 100644
--- a/lib/iris/_concatenate.py
+++ b/lib/iris/_concatenate.py
@@ -722,8 +722,7 @@ def register(self, cube, axis=None, error_on_mismatch=False,
             # Determine whether the fill value requires to be
             # demoted to the default value.
             if fill_value is not None:
-                if cube_signature.fill_value is None or \
-                        cube_signature.fill_value != fill_value:
+                if cube_signature.fill_value != fill_value:
                     # Demote the fill value to the default.
                     self._cube_signature.fill_value = None
             coord_signature = _CoordSignature(cube_signature)
diff --git a/lib/iris/_merge.py b/lib/iris/_merge.py
index 35121dcfae..6958532d82 100644
--- a/lib/iris/_merge.py
+++ b/lib/iris/_merge.py
@@ -1287,8 +1287,7 @@ def register(self, cube, error_on_mismatch=False):
             # Determine whether the fill value requires to be demoted
             # to the default value.
             if cube_signature.fill_value is not None:
-                if other.fill_value is None or \
-                        cube_signature.fill_value != other.fill_value:
+                if cube_signature.fill_value != other.fill_value:
                     # Demote the fill value to the default.
                     signature = self._build_signature(self._source,
                                                       default_fill_value=True)

From 414c362c2805f7ba8f30d9db909538eab3367a11 Mon Sep 17 00:00:00 2001
From: Bill Little <bill.james.little@gmail.com>
Date: Thu, 4 May 2017 14:52:40 +0100
Subject: [PATCH 7/7] expected_fill_value

---
 lib/iris/tests/test_merge.py | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/lib/iris/tests/test_merge.py b/lib/iris/tests/test_merge.py
index 0a2ab12799..98311ab18d 100644
--- a/lib/iris/tests/test_merge.py
+++ b/lib/iris/tests/test_merge.py
@@ -153,18 +153,19 @@ def _expected_fill_value(fill0, fill1):
         return result
 
     def _check_fill_value(self, result, fill0, fill1):
-        fill_value = self._expected_fill_value(fill0, fill1)
-        if fill_value is None:
-            fill_value = ma.masked_array(0, dtype=result.dtype).fill_value
+        expected_fill_value = self._expected_fill_value(fill0, fill1)
+        if expected_fill_value is None:
             self.assertIsNone(result.fill_value)
             data = result.data
             if ma.isMaskedArray(data):
-                self.assertEqual(data.fill_value, fill_value)
+                np_fill_value = ma.masked_array(0,
+                                                dtype=result.dtype).fill_value
+                self.assertEqual(data.fill_value, np_fill_value)
         else:
-            self.assertEqual(result.fill_value, fill_value)
+            self.assertEqual(result.fill_value, expected_fill_value)
             data = result.data
             if ma.isMaskedArray(data):
-                self.assertEqual(data.fill_value, fill_value)
+                self.assertEqual(data.fill_value, expected_fill_value)
 
     def setUp(self):
         self.dtype = np.dtype('int32')
@@ -201,9 +202,9 @@ def test__masked_masked(self):
                                          fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(0, 1), (0, 1), (0, 1)]
-            fill_value = self._expected_fill_value(fill0, fill1)
+            expected_fill_value = self._expected_fill_value(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
-                                       fill_value=fill_value)
+                                       fill_value=expected_fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
             self.assertEqual(result.dtype, self.dtype)
             self._check_fill_value(result, fill0, fill1)
@@ -219,9 +220,9 @@ def test__ndarray_masked(self):
                                          fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(1, 1), (0, 1), (0, 1)]
-            fill_value = self._expected_fill_value(fill0, fill1)
+            expected_fill_value = self._expected_fill_value(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
-                                       fill_value=fill_value)
+                                       fill_value=expected_fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
             self.assertEqual(result.dtype, self.dtype)
             self._check_fill_value(result, fill0, fill1)
@@ -237,9 +238,9 @@ def test__masked_ndarray(self):
                                          fill_value=fill1))
             result = cubes.merge_cube()
             mask = [(0, 0), (0, 1), (0, 1)]
-            fill_value = self._expected_fill_value(fill0, fill1)
+            expected_fill_value = self._expected_fill_value(fill0, fill1)
             expected = self._make_data([0, 1], mask=mask, dtype=self.dtype,
-                                       fill_value=fill_value)
+                                       fill_value=expected_fill_value)
             self.assertMaskedArrayEqual(result.data, expected)
             self.assertEqual(result.dtype, self.dtype)
             self._check_fill_value(result, fill0, fill1)