Skip to content

Commit

Permalink
Opt out of floor division for float dtype time encoding (pydata#9497)
Browse files Browse the repository at this point in the history
  • Loading branch information
spencerkclark authored and hollymandel committed Sep 23, 2024
1 parent ea3eebf commit e4945c0
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 13 deletions.
5 changes: 4 additions & 1 deletion doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ Bug fixes
- Make illegal path-like variable names when constructing a DataTree from a Dataset
(:issue:`9339`, :pull:`9378`)
By `Etienne Schalk <https://github.com/etienneschalk>`_.

- Fix bug when encoding times with missing values as floats in the case when
the non-missing times could in theory be encoded with integers
(:issue:`9488`, :pull:`9497`). By `Spencer Clark
<https://github.com/spencerkclark>`_.


Documentation
Expand Down
4 changes: 2 additions & 2 deletions xarray/coding/times.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ def _eagerly_encode_cf_datetime(
# needed time delta to encode faithfully to int64
needed_time_delta = _time_units_to_timedelta64(needed_units)

floor_division = True
floor_division = np.issubdtype(dtype, np.integer) or dtype is None
if time_delta > needed_time_delta:
floor_division = False
if dtype is None:
Expand Down Expand Up @@ -892,7 +892,7 @@ def _eagerly_encode_cf_timedelta(
# needed time delta to encode faithfully to int64
needed_time_delta = _time_units_to_timedelta64(needed_units)

floor_division = True
floor_division = np.issubdtype(dtype, np.integer) or dtype is None
if time_delta > needed_time_delta:
floor_division = False
if dtype is None:
Expand Down
42 changes: 32 additions & 10 deletions xarray/tests/test_coding_times.py
Original file line number Diff line number Diff line change
Expand Up @@ -1383,24 +1383,46 @@ def test_roundtrip_timedelta64_nanosecond_precision_warning() -> None:
assert decoded_var.encoding["dtype"] == np.int64


def test_roundtrip_float_times() -> None:
# Regression test for GitHub issue #8271
fill_value = 20.0
times = [
np.datetime64("1970-01-01 00:00:00", "ns"),
np.datetime64("1970-01-01 06:00:00", "ns"),
np.datetime64("NaT", "ns"),
]
_TEST_ROUNDTRIP_FLOAT_TIMES_TESTS = {
"GH-8271": (
20.0,
np.array(
["1970-01-01 00:00:00", "1970-01-01 06:00:00", "NaT"],
dtype="datetime64[ns]",
),
"days since 1960-01-01",
np.array([3653, 3653.25, 20.0]),
),
"GH-9488-datetime64[ns]": (
1.0e20,
np.array(["2010-01-01 12:00:00", "NaT"], dtype="datetime64[ns]"),
"seconds since 2010-01-01",
np.array([43200, 1.0e20]),
),
"GH-9488-timedelta64[ns]": (
1.0e20,
np.array([1_000_000_000, "NaT"], dtype="timedelta64[ns]"),
"seconds",
np.array([1.0, 1.0e20]),
),
}


units = "days since 1960-01-01"
@pytest.mark.parametrize(
("fill_value", "times", "units", "encoded_values"),
_TEST_ROUNDTRIP_FLOAT_TIMES_TESTS.values(),
ids=_TEST_ROUNDTRIP_FLOAT_TIMES_TESTS.keys(),
)
def test_roundtrip_float_times(fill_value, times, units, encoded_values) -> None:
# Regression test for GitHub issues #8271 and #9488
var = Variable(
["time"],
times,
encoding=dict(dtype=np.float64, _FillValue=fill_value, units=units),
)

encoded_var = conventions.encode_cf_variable(var)
np.testing.assert_array_equal(encoded_var, np.array([3653, 3653.25, 20.0]))
np.testing.assert_array_equal(encoded_var, encoded_values)
assert encoded_var.attrs["units"] == units
assert encoded_var.attrs["_FillValue"] == fill_value

Expand Down

0 comments on commit e4945c0

Please sign in to comment.