diff --git a/Changelog.rst b/Changelog.rst index dc36568b30..6c8b66dfbb 100644 --- a/Changelog.rst +++ b/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) diff --git a/cf/field.py b/cf/field.py index 71bc209dc4..5befe1eb53 100644 --- a/cf/field.py +++ b/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" 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..50ea930b34 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,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"):