diff --git a/Changelog.rst b/Changelog.rst index e843cdaee5..f0cf3a3579 100644 --- a/Changelog.rst +++ b/Changelog.rst @@ -9,6 +9,8 @@ version NEXTVERSION (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) +* Fix bug that caused climatological time collapses within/over days + to fail (https://github.com/NCAS-CMS/cf-python/issues/809) * New keyword parameter to `cf.Field.derivative`: ``ignore_coordinate_units`` (https://github.com/NCAS-CMS/cf-python/issues/807) diff --git a/cf/field.py b/cf/field.py index f10362b2f8..f6c90d215e 100644 --- a/cf/field.py +++ b/cf/field.py @@ -8392,7 +8392,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" diff --git a/cf/test/test_TimeDuration.py b/cf/test/test_TimeDuration.py index ce7cf69aa7..e8ae369d4f 100644 --- a/cf/test/test_TimeDuration.py +++ b/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"), diff --git a/cf/test/test_collapse.py b/cf/test/test_collapse.py index 0525aecddb..c7c866c8b6 100644 --- a/cf/test/test_collapse.py +++ b/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 diff --git a/cf/timeduration.py b/cf/timeduration.py index 32c3903ecf..1cf79eb15c 100644 --- a/cf/timeduration.py +++ b/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,15 +736,14 @@ 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__)): + # 'other' is a number or another TimeDuration object => we + # can use the usual binary operation method. return self._binary_operation(other, op, aug_assignment) if may_be_datetime and hasattr(other, "timetuple"): - # other is a date-time object + # 'other' is a date-time object => we must use the special + # datetime arithmetic operation. try: return self._datetime_arithmetic(other, getattr(operator, op)) except TypeError: