Skip to content

Commit

Permalink
fix regression in time-like check when decoding masked data (#8277)
Browse files Browse the repository at this point in the history
* fix regression in time-like check when decoding masked data

* add test

* use _unpack_netcdf_time_units to check for proper units-string

* test with decode_times
  • Loading branch information
kmuehlbauer authored Oct 18, 2023
1 parent e611c97 commit 8f3a302
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 2 deletions.
3 changes: 3 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,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>`_.

- Fix datetime encoding precision loss regression introduced in the previous
release for datetimes encoded with units requiring floating point values, and
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

0 comments on commit 8f3a302

Please sign in to comment.