Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix regression in time-like check when decoding masked data #8277

Merged
merged 4 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ Bug fixes
- :py:meth:`DataArray.rename` & :py:meth:`Dataset.rename` would emit a warning
when the operation was a no-op. (:issue:`8266`)
By `Simon Hansen <https://github.com/hoxbro>`_.
- Fixed a regression introduced in the previous release checking time-like units
when encoding/decoding masked data (:issue:`8269`, :pull:`8277`).
By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.


Documentation
Expand Down
17 changes: 15 additions & 2 deletions xarray/coding/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,9 @@ def _apply_mask(

def _is_time_like(units):
# test for time-like
if units is None:
return False
time_strings = [
"since",
"days",
"hours",
"minutes",
Expand All @@ -227,7 +228,19 @@ def _is_time_like(units):
"microseconds",
"nanoseconds",
]
return any(tstr in str(units) for tstr in time_strings)
units = str(units)
# to prevent detecting units like `days accumulated` as time-like
# special casing for datetime-units and timedelta-units (GH-8269)
if "since" in units:
from xarray.coding.times import _unpack_netcdf_time_units

try:
_unpack_netcdf_time_units(units)
except ValueError:
return False
return True
else:
return any(tstr == units for tstr in time_strings)


class CFMaskCoder(VariableCoder):
Expand Down
11 changes: 11 additions & 0 deletions xarray/tests/test_conventions.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,17 @@ def test_invalid_time_units_raises_eagerly(self) -> None:
with pytest.raises(ValueError, match=r"unable to decode time"):
decode_cf(ds)

@pytest.mark.parametrize("decode_times", [True, False])
def test_invalid_timedelta_units_do_not_decode(self, decode_times) -> None:
# regression test for #8269
ds = Dataset(
{"time": ("time", [0, 1, 20], {"units": "days invalid", "_FillValue": 20})}
)
expected = Dataset(
{"time": ("time", [0.0, 1.0, np.nan], {"units": "days invalid"})}
)
assert_identical(expected, decode_cf(ds, decode_times=decode_times))

@requires_cftime
def test_dataset_repr_with_netcdf4_datetimes(self) -> None:
# regression test for #347
Expand Down
Loading