From e8aa5c5235803ee7992b9486137ac548723f1af1 Mon Sep 17 00:00:00 2001 From: Stefan Bender Date: Wed, 10 Apr 2024 16:08:23 +0200 Subject: [PATCH 1/4] Keep attributes for "bounds" variables Issue #2921 is about mismatching time units between a time variable and its "bounds" companion. However, #2965 does more than fixing #2921, it removes all double attributes from "bounds" variables which has the undesired side effect that there is currently no way to save them to netcdf with xarray. Since the mentioned link is a recommendation and not a hard requirement for CF compliance, these attributes should be left to the caller to prepare the dataset variables appropriately if required. Reduces the amount of surprise that attributes are not written to disk and fixes #8368. --- xarray/conventions.py | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/xarray/conventions.py b/xarray/conventions.py index 6eff45c5b2d..e6fab022aa3 100644 --- a/xarray/conventions.py +++ b/xarray/conventions.py @@ -794,24 +794,4 @@ def cf_encoder(variables: T_Variables, attributes: T_Attrs): new_vars = {k: encode_cf_variable(v, name=k) for k, v in variables.items()} - # Remove attrs from bounds variables (issue #2921) - for var in new_vars.values(): - bounds = var.attrs["bounds"] if "bounds" in var.attrs else None - if bounds and bounds in new_vars: - # see http://cfconventions.org/cf-conventions/cf-conventions.html#cell-boundaries - for attr in [ - "units", - "standard_name", - "axis", - "positive", - "calendar", - "long_name", - "leap_month", - "leap_year", - "month_lengths", - ]: - if attr in new_vars[bounds].attrs and attr in var.attrs: - if new_vars[bounds].attrs[attr] == var.attrs[attr]: - new_vars[bounds].attrs.pop(attr) - return new_vars, attributes From 8c1b53929d711dbcbc8d030f6950b23ddb05c3e8 Mon Sep 17 00:00:00 2001 From: Stefan Bender Date: Tue, 21 May 2024 11:11:55 +0200 Subject: [PATCH 2/4] tests: Remove attributes checks for time bounds After encoding these and removing the removal of attributes from time bounds variables, these should be in the attributes. Removes the checks that they aren't. --- xarray/tests/test_coding_times.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/xarray/tests/test_coding_times.py b/xarray/tests/test_coding_times.py index 9a5589ff872..4ed224f4bee 100644 --- a/xarray/tests/test_coding_times.py +++ b/xarray/tests/test_coding_times.py @@ -732,15 +732,11 @@ def test_encode_time_bounds() -> None: encoded, _ = cf_encoder(ds.variables, ds.attrs) assert_equal(encoded["time_bounds"], expected["time_bounds"]) - assert "calendar" not in encoded["time_bounds"].attrs - assert "units" not in encoded["time_bounds"].attrs # if time_bounds attrs are same as time attrs, it doesn't matter ds.time_bounds.encoding = {"calendar": "noleap", "units": "days since 2000-01-01"} encoded, _ = cf_encoder({k: v for k, v in ds.variables.items()}, ds.attrs) assert_equal(encoded["time_bounds"], expected["time_bounds"]) - assert "calendar" not in encoded["time_bounds"].attrs - assert "units" not in encoded["time_bounds"].attrs # for CF-noncompliant case of time_bounds attrs being different from # time attrs; preserve them for faithful roundtrip @@ -748,7 +744,6 @@ def test_encode_time_bounds() -> None: encoded, _ = cf_encoder({k: v for k, v in ds.variables.items()}, ds.attrs) with pytest.raises(AssertionError): assert_equal(encoded["time_bounds"], expected["time_bounds"]) - assert "calendar" not in encoded["time_bounds"].attrs assert encoded["time_bounds"].attrs["units"] == ds.time_bounds.encoding["units"] ds.time.encoding = {} From ba83823576c853342ad8bb8c2482bc18570d3cc4 Mon Sep 17 00:00:00 2001 From: Stefan Bender Date: Tue, 21 May 2024 12:21:00 +0200 Subject: [PATCH 3/4] tests: Check different `time_bounds` calendar Makes the `time_bounds` calendar really different from `time` and checks that they are correctly encoded to be non-CF compliant. --- xarray/tests/test_coding_times.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xarray/tests/test_coding_times.py b/xarray/tests/test_coding_times.py index 4ed224f4bee..f5db09d27df 100644 --- a/xarray/tests/test_coding_times.py +++ b/xarray/tests/test_coding_times.py @@ -740,10 +740,12 @@ def test_encode_time_bounds() -> None: # for CF-noncompliant case of time_bounds attrs being different from # time attrs; preserve them for faithful roundtrip - ds.time_bounds.encoding = {"calendar": "noleap", "units": "days since 1849-01-01"} + ds.time_bounds.encoding = {"calendar": "gregorian", "units": "days since 1849-01-01"} encoded, _ = cf_encoder({k: v for k, v in ds.variables.items()}, ds.attrs) with pytest.raises(AssertionError): assert_equal(encoded["time_bounds"], expected["time_bounds"]) + assert encoded["time"].attrs["calendar"] == ds.time.encoding["calendar"] + assert encoded["time_bounds"].attrs["calendar"] == ds.time_bounds.encoding["calendar"] assert encoded["time_bounds"].attrs["units"] == ds.time_bounds.encoding["units"] ds.time.encoding = {} From a011c47189446afdb048a1000e70c8f3e4853c51 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 May 2024 10:28:11 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- xarray/tests/test_coding_times.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/xarray/tests/test_coding_times.py b/xarray/tests/test_coding_times.py index f5db09d27df..c168993616f 100644 --- a/xarray/tests/test_coding_times.py +++ b/xarray/tests/test_coding_times.py @@ -740,12 +740,17 @@ def test_encode_time_bounds() -> None: # for CF-noncompliant case of time_bounds attrs being different from # time attrs; preserve them for faithful roundtrip - ds.time_bounds.encoding = {"calendar": "gregorian", "units": "days since 1849-01-01"} + ds.time_bounds.encoding = { + "calendar": "gregorian", + "units": "days since 1849-01-01", + } encoded, _ = cf_encoder({k: v for k, v in ds.variables.items()}, ds.attrs) with pytest.raises(AssertionError): assert_equal(encoded["time_bounds"], expected["time_bounds"]) assert encoded["time"].attrs["calendar"] == ds.time.encoding["calendar"] - assert encoded["time_bounds"].attrs["calendar"] == ds.time_bounds.encoding["calendar"] + assert ( + encoded["time_bounds"].attrs["calendar"] == ds.time_bounds.encoding["calendar"] + ) assert encoded["time_bounds"].attrs["units"] == ds.time_bounds.encoding["units"] ds.time.encoding = {}