within_days collapse

Author: davidhassell

Changelog.rst

@@ -6,7 +6,9 @@ version NEXTVERSION
 * New method: `cf.Field.is_discrete_axis`
   (https://github.com/NCAS-CMS/cf-python/issues/784)
 * Include the UM version as a field property when reading UM files
-  (https://github.com/NCAS-CMS/cf-python/issues/777)
+  (https://github.com/NCAS-CMS/cf-python/issues/809)
+* Fix bug that caused climatological time collapses within/over days
+  to fail (https://github.com/NCAS-CMS/cf-python/issues/725)
 * Fix bug where `cf.example_fields` returned a `list` of Fields rather
   than a `Fieldlist`
   (https://github.com/NCAS-CMS/cf-python/issues/725)

cf/field.py

@@ -8380,7 +8380,6 @@ def _group_weights(weights, iaxis, index):
                         within_days.Units.istime
                         and TimeDuration(24, "hours") % within_days
                     ):
-                        # % Data(1, 'day'): # % within_days:
                         raise ValueError(
                             f"Can't collapse: within_days={within_days!r} "
                             "is not an exact factor of 1 day"

cf/test/test_TimeDuration.py

@@ -107,7 +107,18 @@ def test_TimeDuration(self):
         self.assertEqual(
             cf.TimeDuration(36, "calendar_months") // 8.25, cf.M(4.0)
         )
-        self.assertEqual(cf.TimeDuration(36, "calendar_months") % 10, cf.M(6))
+
+        for y in (
+            10,
+            cf.Data(10, "calendar_months"),
+            cf.TimeDuration(10, "calendar_months"),
+        ):
+            self.assertEqual(
+                cf.TimeDuration(36, "calendar_months") % y, cf.M(6)
+            )
+
+        for y in (10, cf.Data(10, "hours"), cf.h(10), cf.D(5 / 12), cf.m(600)):
+            self.assertEqual(cf.h(36) % y, cf.h(6))
 
         self.assertEqual(
             cf.TimeDuration(24, "hours") + cf.TimeDuration(0.5, "days"),

cf/test/test_collapse.py

@@ -246,6 +246,23 @@ def test_Field_collapse_CLIMATOLOGICAL_TIME(self):
             print(g.constructs)
         self.assertEqual(list(g.shape), expected_shape)
 
+    def test_Field_collapse_CLIMATOLOGICAL_TIME_within_days(self):
+        verbose = False
+
+        f = cf.example_field(5)
+
+        g = f.collapse(
+            "T: mean within days time: minimum over days", within_days=cf.h(12)
+        )
+        expected_shape = list(f.shape)
+        expected_shape[0] = 2
+
+        if verbose:
+            print("\n", f)
+            print(g)
+            print(g.constructs)
+        self.assertEqual(list(g.shape), expected_shape)
+
     def test_Field_collapse(self):
         verbose = False
 

cf/timeduration.py

@@ -696,7 +696,6 @@ def _apply_binary_comparison(self, other, op):
             op: `str`, the binary comparison operator to apply.
 
         """
-
         if isinstance(other, (self.__class__, int, float)):
             return bool(self._binary_operation(other, op))
 
@@ -737,11 +736,7 @@ def _apply_binary_arithmetic(
                 do not skip.
 
         """
-        check_simple_types = [int, float]
-        if may_be_datetime:
-            check_simple_types.append(self.__class__)
-
-        if isinstance(other, tuple(check_simple_types)):
+        if isinstance(other, (int, float, self.__class__)):
             return self._binary_operation(other, op, aug_assignment)
 
         if may_be_datetime and hasattr(other, "timetuple"):