-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Following pandas 3.0, make Day
cftime offset non-Tick
-like
#10650
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
base: main
Are you sure you want to change the base?
Changes from all commits
ea43798
f897d20
b87bb35
a830e04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -445,7 +445,6 @@ def test_eq(a, b): | |
(Second(), 3, Second(n=3)), | ||
(Millisecond(), 3, Millisecond(n=3)), | ||
(Microsecond(), 3, Microsecond(n=3)), | ||
(Day(), 0.5, Hour(n=12)), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that
A similar error is raised now for the cftime version of this offset. This is tested in b87bb35. |
||
(Hour(), 0.5, Minute(n=30)), | ||
(Hour(), -0.5, Minute(n=-30)), | ||
(Minute(), 0.5, Second(n=30)), | ||
|
@@ -472,7 +471,15 @@ def test_mul_float_multiple_next_higher_resolution(): | |
|
||
@pytest.mark.parametrize( | ||
"offset", | ||
[YearBegin(), YearEnd(), QuarterBegin(), QuarterEnd(), MonthBegin(), MonthEnd()], | ||
[ | ||
YearBegin(), | ||
YearEnd(), | ||
QuarterBegin(), | ||
QuarterEnd(), | ||
MonthBegin(), | ||
MonthEnd(), | ||
Day(), | ||
], | ||
ids=_id_func, | ||
) | ||
def test_nonTick_offset_multiplied_float_error(offset): | ||
|
@@ -534,6 +541,20 @@ def test_add_sub_monthly(offset, expected_date_args, calendar): | |
assert result == expected | ||
|
||
|
||
def test_add_daily_offsets() -> None: | ||
offset = Day(n=2) | ||
expected = Day(n=4) | ||
result = offset + offset | ||
assert result == expected | ||
|
||
|
||
def test_subtract_daily_offsets() -> None: | ||
offset = Day(n=2) | ||
expected = Day(n=0) | ||
result = offset - offset | ||
assert result == expected | ||
|
||
|
||
@pytest.mark.parametrize(("offset", "expected_date_args"), _ADD_TESTS, ids=_id_func) | ||
def test_radd_sub_monthly(offset, expected_date_args, calendar): | ||
date_type = get_date_type(calendar) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -830,7 +830,6 @@ def test_cftimeindex_add_timedeltaindex(calendar) -> None: | |
@pytest.mark.parametrize( | ||
"freq,units", | ||
[ | ||
("D", "D"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that
A similar error is raised now for attempting to shift a |
||
("h", "h"), | ||
("min", "min"), | ||
("s", "s"), | ||
|
@@ -856,7 +855,7 @@ def test_cftimeindex_shift_float_us() -> None: | |
|
||
|
||
@requires_cftime | ||
@pytest.mark.parametrize("freq", ["YS", "YE", "QS", "QE", "MS", "ME"]) | ||
@pytest.mark.parametrize("freq", ["YS", "YE", "QS", "QE", "MS", "ME", "D"]) | ||
def test_cftimeindex_shift_float_fails_for_non_tick_freqs(freq) -> None: | ||
a = xr.date_range("2000", periods=3, freq="D", use_cftime=True) | ||
with pytest.raises(TypeError, match="unsupported operand type"): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,12 +6,17 @@ | |
import numpy as np | ||
import pandas as pd | ||
import pytest | ||
from packaging.version import Version | ||
|
||
import xarray as xr | ||
from xarray.coding.cftime_offsets import _new_to_legacy_freq | ||
from xarray.coding.cftime_offsets import ( | ||
CFTIME_TICKS, | ||
Day, | ||
_new_to_legacy_freq, | ||
to_offset, | ||
) | ||
from xarray.coding.cftimeindex import CFTimeIndex | ||
from xarray.core.resample_cftime import CFTimeGrouper | ||
from xarray.tests import has_pandas_3 | ||
|
||
cftime = pytest.importorskip("cftime") | ||
|
||
|
@@ -54,6 +59,20 @@ | |
] | ||
|
||
|
||
def has_tick_resample_freq(freqs): | ||
resample_freq, _ = freqs | ||
resample_freq_as_offset = to_offset(resample_freq) | ||
return isinstance(resample_freq_as_offset, CFTIME_TICKS) | ||
|
||
|
||
def has_non_tick_resample_freq(freqs): | ||
return not has_tick_resample_freq(freqs) | ||
|
||
|
||
FREQS_WITH_TICK_RESAMPLE_FREQ = list(filter(has_tick_resample_freq, FREQS)) | ||
FREQS_WITH_NON_TICK_RESAMPLE_FREQ = list(filter(has_non_tick_resample_freq, FREQS)) | ||
|
||
|
||
def compare_against_pandas( | ||
da_datetimeindex, | ||
da_cftimeindex, | ||
|
@@ -110,22 +129,14 @@ def da(index) -> xr.DataArray: | |
) | ||
|
||
|
||
@pytest.mark.parametrize("freqs", FREQS, ids=lambda x: "{}->{}".format(*x)) | ||
@pytest.mark.parametrize( | ||
"freqs", FREQS_WITH_TICK_RESAMPLE_FREQ, ids=lambda x: "{}->{}".format(*x) | ||
) | ||
@pytest.mark.parametrize("closed", [None, "left", "right"]) | ||
@pytest.mark.parametrize("label", [None, "left", "right"]) | ||
@pytest.mark.parametrize("offset", [None, "5s"], ids=lambda x: f"{x}") | ||
def test_resample(freqs, closed, label, offset) -> None: | ||
def test_resample_with_tick_resample_freq(freqs, closed, label, offset) -> None: | ||
initial_freq, resample_freq = freqs | ||
if ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since our minimum version of pandas is greater than or equal to 2.2, I took the opportunity to remove this old test-skipping logic. |
||
resample_freq == "4001D" | ||
and closed == "right" | ||
and Version(pd.__version__) < Version("2.2") | ||
): | ||
pytest.skip( | ||
"Pandas fixed a bug in this test case in version 2.2, which we " | ||
"ported to xarray, so this test no longer produces the same " | ||
"result as pandas for earlier pandas versions." | ||
) | ||
start = "2000-01-01T12:07:01" | ||
origin = "start" | ||
|
||
|
@@ -149,6 +160,43 @@ def test_resample(freqs, closed, label, offset) -> None: | |
) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"freqs", FREQS_WITH_NON_TICK_RESAMPLE_FREQ, ids=lambda x: "{}->{}".format(*x) | ||
) | ||
@pytest.mark.parametrize("closed", [None, "left", "right"]) | ||
@pytest.mark.parametrize("label", [None, "left", "right"]) | ||
def test_resample_with_non_tick_resample_freq(freqs, closed, label) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the |
||
initial_freq, resample_freq = freqs | ||
resample_freq_as_offset = to_offset(resample_freq) | ||
if isinstance(resample_freq_as_offset, Day) and not has_pandas_3: | ||
pytest.skip("Only valid for pandas >= 3.0") | ||
start = "2000-01-01T12:07:01" | ||
|
||
# Set offset and origin to their default values since they have no effect | ||
# on resampling data with a non-tick resample frequency. | ||
offset = None | ||
origin = "start_day" | ||
|
||
datetime_index = pd.date_range( | ||
start=start, periods=5, freq=_new_to_legacy_freq(initial_freq) | ||
) | ||
cftime_index = xr.date_range( | ||
start=start, periods=5, freq=initial_freq, use_cftime=True | ||
) | ||
da_datetimeindex = da(datetime_index) | ||
da_cftimeindex = da(cftime_index) | ||
|
||
compare_against_pandas( | ||
da_datetimeindex, | ||
da_cftimeindex, | ||
resample_freq, | ||
closed=closed, | ||
label=label, | ||
offset=offset, | ||
origin=origin, | ||
) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
("freq", "expected"), | ||
[ | ||
|
@@ -228,7 +276,7 @@ def test_invalid_offset_error(offset: str | int) -> None: | |
cftime_index = xr.date_range("2000", periods=5, use_cftime=True) | ||
da_cftime = da(cftime_index) | ||
with pytest.raises(ValueError, match="offset must be"): | ||
da_cftime.resample(time="2D", offset=offset) # type: ignore[arg-type] | ||
da_cftime.resample(time="2h", offset=offset) # type: ignore[arg-type] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems like a major break. I've certainly used |
||
|
||
|
||
def test_timedelta_offset() -> None: | ||
|
@@ -238,6 +286,15 @@ def test_timedelta_offset() -> None: | |
cftime_index = xr.date_range("2000", periods=5, use_cftime=True) | ||
da_cftime = da(cftime_index) | ||
|
||
timedelta_result = da_cftime.resample(time="2D", offset=timedelta).mean() | ||
string_result = da_cftime.resample(time="2D", offset=string).mean() | ||
timedelta_result = da_cftime.resample(time="2h", offset=timedelta).mean() | ||
string_result = da_cftime.resample(time="2h", offset=string).mean() | ||
Comment on lines
+289
to
+290
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switched to |
||
xr.testing.assert_identical(timedelta_result, string_result) | ||
|
||
|
||
@pytest.mark.parametrize(("option", "value"), [("offset", "5s"), ("origin", "start")]) | ||
def test_non_tick_option_warning(option, value) -> None: | ||
cftime_index = xr.date_range("2000", periods=5, use_cftime=True) | ||
da_cftime = da(cftime_index) | ||
kwargs = {option: value} | ||
with pytest.warns(RuntimeWarning, match=option): | ||
da_cftime.resample(time="ME", **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.