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 time indexing regression in convert_calendar #9192

Merged
merged 8 commits into from
Jul 11, 2024
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
6 changes: 6 additions & 0 deletions doc/whats-new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ Bug fixes
By `Justus Magin <https://github.com/keewis>`_.
- Promote floating-point numeric datetimes before decoding (:issue:`9179`, :pull:`9182`).
By `Justus Magin <https://github.com/keewis>`_.
- Address regression introduced in :pull:`9002` that prevented objects returned
by py:meth:`DataArray.convert_calendar` to be indexed by a time index in
certain circumstances (:issue:`9138`, :pull:`9192`). By `Mark Harfouche
<https://github.com/hmaarrfk>`_ and `Spencer Clark
<https://github.com/spencerkclark>`.

- Fiy static typing of tolerance arguments by allowing `str` type (:issue:`8892`, :pull:`9194`).
By `Michael Niklas <https://github.com/headtr1ck>`_.
- Dark themes are now properly detected for ``html[data-theme=dark]``-tags (:pull:`9200`).
Expand Down
12 changes: 11 additions & 1 deletion xarray/coding/calendar_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@

from xarray.coding.cftime_offsets import date_range_like, get_date_type
from xarray.coding.cftimeindex import CFTimeIndex
from xarray.coding.times import _should_cftime_be_used, convert_times
from xarray.coding.times import (
_should_cftime_be_used,
convert_times,
)
from xarray.core.common import _contains_datetime_like_objects, is_np_datetime_like

try:
Expand Down Expand Up @@ -222,6 +225,13 @@ def convert_calendar(
# Remove NaN that where put on invalid dates in target calendar
out = out.where(out[dim].notnull(), drop=True)

if use_cftime:
# Reassign times to ensure time index of output is a CFTimeIndex
# (previously it was an Index due to the presence of NaN values).
# Note this is not needed in the case that the output time index is
# a DatetimeIndex, since DatetimeIndexes can handle NaN values.
out[dim] = CFTimeIndex(out[dim].data)

if missing is not None:
time_target = date_range_like(time, calendar=calendar, use_cftime=use_cftime)
out = out.reindex({dim: time_target}, fill_value=missing)
Expand Down
25 changes: 24 additions & 1 deletion xarray/tests/test_calendar_ops.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from __future__ import annotations

import numpy as np
import pandas as pd
import pytest

from xarray import DataArray, infer_freq
from xarray import CFTimeIndex, DataArray, infer_freq
from xarray.coding.calendar_ops import convert_calendar, interp_calendar
from xarray.coding.cftime_offsets import date_range
from xarray.testing import assert_identical
Expand Down Expand Up @@ -286,3 +287,25 @@ def test_interp_calendar_errors():
ValueError, match="Both 'source.x' and 'target' must contain datetime objects."
):
interp_calendar(da1, da2, dim="x")


@requires_cftime
@pytest.mark.parametrize(
("source_calendar", "target_calendar", "expected_index"),
[("standard", "noleap", CFTimeIndex), ("all_leap", "standard", pd.DatetimeIndex)],
)
def test_convert_calendar_produces_time_index(
source_calendar, target_calendar, expected_index
):
# https://github.com/pydata/xarray/issues/9138
time = date_range("2000-01-01", "2002-01-01", freq="D", calendar=source_calendar)
temperature = np.ones(len(time))
da = DataArray(
data=temperature,
dims=["time"],
coords=dict(
time=time,
),
)
converted = da.convert_calendar(target_calendar)
assert isinstance(converted.indexes["time"], expected_index)
Loading