From 69c1a29253ea0b4e4a146686b1112852a3f0a4a9 Mon Sep 17 00:00:00 2001 From: Jiwoo Lee Date: Wed, 5 Jun 2024 22:32:07 -0700 Subject: [PATCH 1/6] make sure time diff type is timedelta: in case it was timedelta64 convert it, so following `.seconds` line could run without error --- xcdat/bounds.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xcdat/bounds.py b/xcdat/bounds.py index 13dd6e43..65ddc9ce 100644 --- a/xcdat/bounds.py +++ b/xcdat/bounds.py @@ -603,7 +603,10 @@ def _create_time_bounds( elif freq == "hour": # Determine the daily frequency for generating time bounds. if daily_subfreq is None: - diff = time.values[1] - time.values[0] + diff = pd.to_timedelta(time.values[1] - time.values[0]) + # `cftime` objects only support arithmetic using `timedelta` objects, so + # the values of `diffs` must be casted from `dtype="timedelta64[ns]"` + # to `timedelta` objects. hrs = diff.seconds / 3600 daily_subfreq = int(24 / hrs) # type: ignore From a60007f1708c5ed6bc09818d55a158064ff464da Mon Sep 17 00:00:00 2001 From: Jiwoo Lee Date: Thu, 6 Jun 2024 14:00:25 -0700 Subject: [PATCH 2/6] Update xcdat/bounds.py Co-authored-by: Stephen Po-Chedley --- xcdat/bounds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcdat/bounds.py b/xcdat/bounds.py index 65ddc9ce..bfa93156 100644 --- a/xcdat/bounds.py +++ b/xcdat/bounds.py @@ -603,10 +603,10 @@ def _create_time_bounds( elif freq == "hour": # Determine the daily frequency for generating time bounds. if daily_subfreq is None: - diff = pd.to_timedelta(time.values[1] - time.values[0]) # `cftime` objects only support arithmetic using `timedelta` objects, so # the values of `diffs` must be casted from `dtype="timedelta64[ns]"` # to `timedelta` objects. + diff = pd.to_timedelta(time.values[1] - time.values[0]) hrs = diff.seconds / 3600 daily_subfreq = int(24 / hrs) # type: ignore From c019318152b42a26ab05eb6e58f3e7c3e277e1e0 Mon Sep 17 00:00:00 2001 From: Jiwoo Lee Date: Thu, 6 Jun 2024 14:32:21 -0700 Subject: [PATCH 3/6] proceed `pd.to_timedelta` only if diff type is `np.timedelta64` --- xcdat/bounds.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/xcdat/bounds.py b/xcdat/bounds.py index bfa93156..50b727e1 100644 --- a/xcdat/bounds.py +++ b/xcdat/bounds.py @@ -603,10 +603,12 @@ def _create_time_bounds( elif freq == "hour": # Determine the daily frequency for generating time bounds. if daily_subfreq is None: - # `cftime` objects only support arithmetic using `timedelta` objects, so - # the values of `diffs` must be casted from `dtype="timedelta64[ns]"` - # to `timedelta` objects. - diff = pd.to_timedelta(time.values[1] - time.values[0]) + diff = time.values[1] - time.values[0] + if isinstance(diff, np.timedelta64): + diff = pd.to_timedelta(diff) + # `cftime` objects only support arithmetic using `timedelta` objects, so + # the values of `diffs` must be casted from `dtype="timedelta64[ns]"` + # to `timedelta` objects. hrs = diff.seconds / 3600 daily_subfreq = int(24 / hrs) # type: ignore From 09203f25a6c7f9fdc06231a3a5e14fd8a55761bb Mon Sep 17 00:00:00 2001 From: Tom Vo Date: Thu, 6 Jun 2024 15:38:02 -0700 Subject: [PATCH 4/6] Update `_create_time_bounds()` - Add comment about needing to convert dype=timedelta64[ns] to pandas timedelta object - Ignore C901 flake8 error --- xcdat/bounds.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/xcdat/bounds.py b/xcdat/bounds.py index 50b727e1..38e7f6ff 100644 --- a/xcdat/bounds.py +++ b/xcdat/bounds.py @@ -504,7 +504,7 @@ def _get_bounds_keys(self, axis: CFAxisKey) -> List[str]: return list(set(keys)) - def _create_time_bounds( + def _create_time_bounds( # noqa: C901 self, time: xr.DataArray, freq: Optional[Literal["year", "month", "day", "hour"]] = None, @@ -601,14 +601,16 @@ def _create_time_bounds( elif freq == "day": time_bnds = self._create_daily_time_bounds(timesteps, obj_type) elif freq == "hour": - # Determine the daily frequency for generating time bounds. + # Determine the daily frequency for generating time bounds. if daily_subfreq is None: diff = time.values[1] - time.values[0] + + # Arrays with `dtype="timedelta64[ns]"` must be converted to + # pandas timedelta objects in order to access the `.seconds` + # time component. if isinstance(diff, np.timedelta64): diff = pd.to_timedelta(diff) - # `cftime` objects only support arithmetic using `timedelta` objects, so - # the values of `diffs` must be casted from `dtype="timedelta64[ns]"` - # to `timedelta` objects. + hrs = diff.seconds / 3600 daily_subfreq = int(24 / hrs) # type: ignore From c6b314a1c7e33df200a3bba2bd0e73ae0748acad Mon Sep 17 00:00:00 2001 From: Tom Vo Date: Mon, 10 Jun 2024 14:49:08 -0700 Subject: [PATCH 5/6] Add coverage for new code --- tests/fixtures.py | 54 +++++++++++++++++++++++++++++++++++++++++--- tests/test_bounds.py | 15 ++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/tests/fixtures.py b/tests/fixtures.py index 4da71a7e..81f594bb 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -92,6 +92,22 @@ "standard_name": "time", }, ) +time_hourly_dt = xr.DataArray( + data=np.array( + [ + "2000-01-01T00:00:00.000000000", + "2000-01-01T01:00:00.000000000", + "2000-01-01T02:00:00.000000000", + ], + dtype="datetime64[ns]", + ), + dims=["time"], + attrs={ + "axis": "T", + "long_name": "time", + "standard_name": "time", + }, +) time_subhourly = xr.DataArray( data=np.array( [ @@ -189,6 +205,30 @@ "xcdat_bounds": "True", }, ) +time_bnds_hourly_dt = xr.DataArray( + name="time_bnds", + data=np.array( + [ + [ + "2000-01-01T00:00:00.000000000", + "2000-01-01T01:00:00.000000000", + ], + [ + "2000-01-01T01:00:00.000000000", + "2000-01-01T02:00:00.000000000", + ], + [ + "2000-01-01T02:00:00.000000000", + "2000-01-01T03:00:00.000000000", + ], + ], + dtype="datetime64[ns]", + ), + dims=["time", "bnds"], + attrs={ + "xcdat_bounds": "True", + }, +) time_bnds_subhourly = xr.DataArray( name="time_bnds", data=np.array( @@ -495,7 +535,8 @@ def generate_dataset( def generate_dataset_by_frequency( - freq: Literal["subhour", "hour", "day", "month", "year"] = "month" + freq: Literal["subhour", "hour", "day", "month", "year"] = "month", + obj_type: Literal["cftime", "datetime"] = "cftime", ) -> xr.Dataset: """Generates a dataset for a given temporal frequency. @@ -523,8 +564,15 @@ def generate_dataset_by_frequency( time = time_daily.copy() time_bnds = time_bnds_daily.copy() elif freq == "hour": - time = time_hourly.copy() - time_bnds = time_bnds_hourly.copy() + # Test cftime and datetime. datetime subtraction results in + # dtype=timedelta64[ns] objects, which need to be converted to Pandas + # TimeDelta objects to use the `.seconds` time component. + if obj_type == "cftime": + time = time_hourly.copy() + time_bnds = time_bnds_hourly.copy() + else: + time = time_hourly_dt.copy() + time_bnds = time_bnds_hourly_dt.copy() elif freq == "subhour": time = time_subhourly.copy() time_bnds = time_bnds_subhourly.copy() diff --git a/tests/test_bounds.py b/tests/test_bounds.py index 4e2fd028..2952b05e 100644 --- a/tests/test_bounds.py +++ b/tests/test_bounds.py @@ -826,6 +826,21 @@ def test_add_bounds_for_time_coords_with_different_frequencies(self): assert monthly_bounds.identical(ds_monthly_with_bnds) assert yearly_bounds.identical(ds_yearly_with_bnds) + def test_add_bounds_for_hourly_time_coords_as_datetime_bojects(self): + # get reference datasets + ds_hrly_with_bnds = generate_dataset_by_frequency("hour", "datetime") + + # drop bounds for testing + ds_hrly_wo_bnds = ds_hrly_with_bnds.drop_vars("time_bnds") + + # test adding bounds + hourly_bounds = ds_hrly_wo_bnds.bounds.add_time_bounds( + method="freq", freq="hour" + ) + + # ensure identical + assert hourly_bounds.identical(ds_hrly_with_bnds) + def test_add_monthly_bounds_for_end_of_month_set_to_true(self): ds_with_bnds = self.ds_with_bnds.copy() From 189e526ee162930a9ae196a4a09af8a2664e74ed Mon Sep 17 00:00:00 2001 From: Tom Vo Date: Mon, 10 Jun 2024 14:54:12 -0700 Subject: [PATCH 6/6] Update tests/test_bounds.py --- tests/test_bounds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_bounds.py b/tests/test_bounds.py index 2952b05e..fc864a44 100644 --- a/tests/test_bounds.py +++ b/tests/test_bounds.py @@ -826,7 +826,7 @@ def test_add_bounds_for_time_coords_with_different_frequencies(self): assert monthly_bounds.identical(ds_monthly_with_bnds) assert yearly_bounds.identical(ds_yearly_with_bnds) - def test_add_bounds_for_hourly_time_coords_as_datetime_bojects(self): + def test_add_bounds_for_hourly_time_coords_as_datetime_objects(self): # get reference datasets ds_hrly_with_bnds = generate_dataset_by_frequency("hour", "datetime")