diff --git a/Changelog.rst b/Changelog.rst index e698df4f48..546aa97ecf 100644 --- a/Changelog.rst +++ b/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) ---- diff --git a/cf/mixin/fielddomain.py b/cf/mixin/fielddomain.py index 85cf503e55..3e1ea3ae83 100644 --- a/cf/mixin/fielddomain.py +++ b/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 ): diff --git a/cf/test/test_Domain.py b/cf/test/test_Domain.py index b548c553a0..9e5566eab0 100644 --- a/cf/test/test_Domain.py +++ b/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()) diff --git a/cf/test/test_Field.py b/cf/test/test_Field.py index 13b36bf426..4a7a46db6b 100644 --- a/cf/test/test_Field.py +++ b/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)