Merge pull request #763 from sadielbartholomew/del-construct-cyclic-consideration

Override `del_construct` to update cyclic axes set when due

Author: sadielbartholomew

Changelog.rst

@@ -27,6 +27,9 @@ version 3.16.2
 * Fix bug in `cf.aggregate` that sometimes put a null transpose
   operation into the Dask graph when one was not needed
   (https://github.com/NCAS-CMS/cf-python/issues/754)
+* Fix bug whereby `Field.cyclic` is not updated after a
+  `Field.del_construct` operation
+  (https://github.com/NCAS-CMS/cf-python/issues/758)
 
 ----
 

cf/mixin/fielddomain.py

@@ -2408,6 +2408,109 @@ def set_construct(
         # Return the construct key
         return out
 
+    def del_construct(self, *identity, default=ValueError(), **filter_kwargs):
+        """Remove a metadata construct.
+
+        If a domain axis construct is selected for removal then it
+        can't be spanned by any data arrays of the field nor metadata
+        constructs, nor be referenced by any cell method
+        constructs. However, a domain ancillary construct may be
+        removed even if it is referenced by coordinate reference
+        construct.
+
+        .. versionadded:: NEXTVERSION
+
+        .. seealso:: `get_construct`, `constructs`, `has_construct`,
+                     `set_construct`
+
+        :Parameters:
+
+            identity:
+                Select the unique construct that has the identity,
+                defined by its `!identities` method, that matches the
+                given values.
+
+                Additionally, the values are matched against construct
+                identifiers, with or without the ``'key%'`` prefix.
+
+                {{value match}}
+
+                {{displayed identity}}
+
+            default: optional
+                Return the value of the *default* parameter if the
+                data axes have not been set.
+
+                {{default Exception}}
+
+            {{filter_kwargs: optional}}
+
+                .. versionadded:: NEXTVERSION
+
+        :Returns:
+
+                The removed metadata construct.
+
+        **Examples**
+
+        >>> f = {{package}}.example_field(0)
+        >>> print(f)
+        Field: specific_humidity (ncvar%q)
+        ----------------------------------
+        Data            : specific_humidity(latitude(5), longitude(8)) 1
+        Cell methods    : area: mean
+        Dimension coords: latitude(5) = [-75.0, ..., 75.0] degrees_north
+                        : longitude(8) = [22.5, ..., 337.5] degrees_east
+                        : time(1) = [2019-01-01 00:00:00]
+        >>> f.del_construct('time')
+        <{{repr}}DimensionCoordinate: time(1) days since 2018-12-01 >
+        >>> f.del_construct('time')
+        Traceback (most recent call last):
+            ...
+        ValueError: Can't find unique construct to remove
+        >>> f.del_construct('time', default='No time')
+        'No time'
+        >>> f.del_construct('dimensioncoordinate1')
+        <{{repr}}DimensionCoordinate: longitude(8) degrees_east>
+        >>> print(f)
+        Field: specific_humidity (ncvar%q)
+        ----------------------------------
+        Data            : specific_humidity(latitude(5), ncdim%lon(8)) 1
+        Cell methods    : area: mean
+        Dimension coords: latitude(5) = [-75.0, ..., 75.0] degrees_north
+
+        """
+        # Need to re-define to overload this method since cfdm doesn't
+        # have the concept of cyclic axes, so have to update the
+        # register of cyclic axes when we delete a construct in cf.
+
+        # Get the relevant key first because it will be lost upon deletion
+        key = self.construct_key(*identity, default=None, **filter_kwargs)
+        cyclic_axes = self._cyclic
+
+        deld_construct = super().del_construct(
+            *identity, default=None, **filter_kwargs
+        )
+        if deld_construct is None:
+            if default is None:
+                return
+
+            return self._default(
+                default, "Can't find unique construct to remove"
+            )
+
+        # If the construct deleted was a cyclic axes, remove it from the set
+        # of stored cyclic axes, to sync that. This is safe now, since given
+        # the block above we can be sure the deletion was successful.
+        if key in cyclic_axes:
+            # Never change value of _cyclic attribute in-place. Only copy now
+            # when the copy is known to be required.
+            cyclic_axes = cyclic_axes.copy()
+            cyclic_axes.remove(key)
+            self._cyclic = cyclic_axes
+
+        return deld_construct
+
     def set_coordinate_reference(
         self, coordinate_reference, key=None, parent=None, strict=True
     ):

cf/test/test_Domain.py

@@ -391,6 +391,43 @@ def test_Domain_create_regular(self):
             np.allclose(latitude_specific.array - y_points_specific, 0)
         )
 
+    def test_Domain_del_construct(self):
+        """Test the `del_construct` Domain method."""
+        # Test a domain without cyclic axes. These are equivalent tests to
+        # those in the cfdm test suite, to check behaviour is the same in cf.
+        d = self.d.copy()
+
+        self.assertIsInstance(
+            d.del_construct("dimensioncoordinate1"), cf.DimensionCoordinate
+        )
+        self.assertIsInstance(
+            d.del_construct("auxiliarycoordinate1"), cf.AuxiliaryCoordinate
+        )
+        with self.assertRaises(ValueError):
+            d.del_construct("auxiliarycoordinate1")
+
+        self.assertIsNone(
+            d.del_construct("auxiliarycoordinate1", default=None)
+        )
+
+        self.assertIsInstance(d.del_construct("measure:area"), cf.CellMeasure)
+
+        # NOTE: this test will fail presently because of a bug which means
+        # that Field.domain doesn't inherit the cyclic() axes of the
+        # corresponding Field (see Issue #762) which will be fixed shortly.
+        #
+        # Test a domain with cyclic axes, to ensure the cyclic() set is
+        # updated accordingly if a cyclic axes is the one removed.
+        e = cf.example_field(2).domain  # this has a cyclic axes 'domainaxis2'
+        # To delete a cyclic axes, must first delete this dimension coordinate
+        # because 'domainaxis2' spans it.
+        self.assertIsInstance(
+            e.del_construct("dimensioncoordinate2"), cf.DimensionCoordinate
+        )
+        self.assertEqual(e.cyclic(), set(("domainaxis2",)))
+        self.assertIsInstance(e.del_construct("domainaxis2"), cf.DomainAxis)
+        self.assertEqual(e.cyclic(), set())
+
 
 if __name__ == "__main__":
     print("Run date:", datetime.datetime.now())

cf/test/test_Field.py

@@ -2572,6 +2572,37 @@ def test_Field_set_construct_conform(self):
         cm2 = f.cell_method("method:maximum")
         self.assertEqual(cm2.get_axes(), ("T",))
 
+    def test_Field_del_construct(self):
+        """Test the `del_construct` Field method."""
+        # Test a field without cyclic axes. These are equivalent tests to those
+        # in the cfdm test suite, to check the behaviour is the same in cf.
+        f = self.f1.copy()
+
+        self.assertIsInstance(
+            f.del_construct("auxiliarycoordinate1"), cf.AuxiliaryCoordinate
+        )
+
+        with self.assertRaises(ValueError):
+            f.del_construct("auxiliarycoordinate1")
+
+        self.assertIsNone(
+            f.del_construct("auxiliarycoordinate1", default=None)
+        )
+
+        self.assertIsInstance(f.del_construct("measure:area"), cf.CellMeasure)
+
+        # Test a field with cyclic axes, to ensure the cyclic() set is
+        # updated accordingly if a cyclic axes is the one removed.
+        g = cf.example_field(2)  # this has a cyclic axes 'domainaxis2'
+        # To delete a cyclic axes, must first delete this dimension coordinate
+        # because 'domainaxis2' spans it.
+        self.assertIsInstance(
+            g.del_construct("dimensioncoordinate2"), cf.DimensionCoordinate
+        )
+        self.assertEqual(g.cyclic(), set(("domainaxis2",)))
+        self.assertIsInstance(g.del_construct("domainaxis2"), cf.DomainAxis)
+        self.assertEqual(g.cyclic(), set())
+
     def test_Field_persist(self):
         """Test the `persist` Field method."""
         f = cf.example_field(0)