Merge branch 'main' into pad

Author: sadielbartholomew

Changelog.rst

@@ -5,6 +5,11 @@ version 3.17.0
 
 * New methods: `cf.Field.pad_missing` and `cf.Data.pad_missing`
   (https://github.com/NCAS-CMS/cf-python/issues/717)
+* Fix occasional bug when calculating UGRID cell areas when
+  non-spatial coordinates span the discrete axis
+  (https://github.com/NCAS-CMS/cf-python/issues/721)
+* New keyword parameter to `cf.Field.insert_dimension`:
+  ``constructs`` (https://github.com/NCAS-CMS/cf-python/issues/719)
 * Added the ``cell_measures`` and ``coordinates`` keyword arguments to
   `cf.Field.weights`
   (https://github.com/NCAS-CMS/cf-python/issues/709)
@@ -17,6 +22,9 @@ version 3.17.0
 * Fix bug that caused `cf.Field.del_file_location` to fail when
   updating its metdata constructs
   (https://github.com/NCAS-CMS/cf-python/issues/707)
+* Fix bug that caused incorrect data arrays in some cyclic subspaces
+  created by `cf.Field.subspace` and `cf.Field.__getitem__`
+  (https://github.com/NCAS-CMS/cf-python/issues/713)
 
 version 3.16.0
 --------------

cf/field.py

@@ -405,10 +405,12 @@ def __getitem__(self, indices):
                         f"{self.constructs.domain_axis_identity(_)!r} axis"
                     )
 
-                new = new.roll(shift=shift, axis=iaxis)
+                new = new.roll(axis=iaxis, shift=shift)
         else:
             new = self.copy()
 
+        data = new.data
+
         # ------------------------------------------------------------
         # Subspace the field construct's data
         # ------------------------------------------------------------
@@ -8685,7 +8687,9 @@ def _update_cell_methods(
             )  # pragma: no cover
 
     @_inplace_enabled(default=False)
-    def insert_dimension(self, axis, position=0, inplace=False):
+    def insert_dimension(
+        self, axis, position=0, constructs=False, inplace=False
+    ):
         """Insert a size 1 axis into the data array.
 
         .. versionadded:: 3.0.0
@@ -8710,6 +8714,13 @@ def insert_dimension(self, axis, position=0, inplace=False):
                 data array. By default the new axis has position 0, the
                 slowest varying position.
 
+            constructs: `bool`, optional
+                If True then also insert the new axis into all
+                metadata constructs that don't already include it. By
+                default, metadata constructs are not changed.
+
+                .. versionadded:: 3.17.0
+
             {{inplace: `bool`, optional}}
 
         :Returns:
@@ -8753,24 +8764,13 @@ def insert_dimension(self, axis, position=0, inplace=False):
                         : time(1) = [2019-01-01 00:00:00]
 
         """
-        f = _inplace_enabled_define_and_cleanup(self)
-
-        if axis is None:
-            axis = f.set_construct(self._DomainAxis(1))
-        else:
-            axis = f.domain_axis(
-                axis,
-                key=True,
-                default=ValueError("Can't identify a unique axis to insert"),
-            )
-
-        # Expand the dims in the field construct's data array
-        super(Field, f).insert_dimension(
-            axis=axis, position=position, inplace=True
+        return super().insert_dimension(
+            axis=axis,
+            position=position,
+            constructs=constructs,
+            inplace=inplace,
         )
 
-        return f
-
     def indices(self, *mode, **kwargs):
         """Create indices that define a subspace of the field construct.
 

cf/test/test_Field.py

@@ -656,6 +656,13 @@ def test_Field__getitem__(self):
         with self.assertRaises(IndexError):
             f[..., [False] * f.shape[-1]]
 
+        # Test with cyclic subspace
+        f.cyclic("grid_longitude")
+        g = f[:, -3:-5:1]
+        self.assertEqual(g.shape, (10, 7))
+        self.assertTrue(np.allclose(f[:, -3:].array, g[:, :3].array))
+        self.assertTrue(f[:, :4].equals(g[:, 3:]))
+
     def test_Field__setitem__(self):
         f = self.f.copy().squeeze()
 
@@ -1136,6 +1143,10 @@ def test_Field_insert_dimension(self):
         self.assertEqual(g.ndim, f.ndim + 1)
         self.assertEqual(g.get_data_axes()[1:], f.get_data_axes())
 
+        self.assertEqual(g.cell_measure().ndim, 2)
+        h = g.insert_dimension(None, constructs=True)
+        self.assertEqual(h.cell_measure().ndim, 3)
+
         with self.assertRaises(ValueError):
             f.insert_dimension(1, "qwerty")
 

cf/weights.py

@@ -1652,6 +1652,9 @@ def _geometry_ugrid_cells(cls, f, domain_axis, cell_type, auto=False):
         )
 
         for key, aux in auxiliary_coordinates_1d.items():
+            if str(aux.ctype) not in "XYZ":
+                continue
+
             aux_axis = f.get_data_axes(key)[0]
 
             ugrid = f.domain_topology(default=None, filter_by_axis=(aux_axis,))
@@ -1665,8 +1668,8 @@ def _geometry_ugrid_cells(cls, f, domain_axis, cell_type, auto=False):
             ):
                 continue
 
-            # Still here? Then this auxiliary coordinate has either UGRID
-            # or geometry cells.
+            # Still here? Then this X, Y, or Z auxiliary coordinate is
+            # for either UGRID or geometry cells.
             if aux.X:
                 aux_X = aux.copy()
                 x_axis = aux_axis
@@ -1701,7 +1704,7 @@ def _geometry_ugrid_cells(cls, f, domain_axis, cell_type, auto=False):
 
             raise ValueError(
                 "Can't create weights: X and Y cells span different domain "
-                "axes"
+                f"axes: {x_axis} != {y_axis}"
             )
 
         axis = x_axis