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

Adjust tests to use updated pandas syntax for offsets #4537

Merged
merged 9 commits into from
Mar 6, 2021
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
9 changes: 7 additions & 2 deletions xarray/tests/test_dataarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import numpy as np
import pandas as pd
import pytest
from pandas.tseries.frequencies import to_offset

import xarray as xr
from xarray import (
Expand Down Expand Up @@ -2990,9 +2991,13 @@ def test_resample(self):
actual = array.resample(time="24H").reduce(np.mean)
assert_identical(expected, actual)

# Our use of `loffset` may change if we align our API with pandas' changes.
# ref https://github.com/pydata/xarray/pull/4537
actual = array.resample(time="24H", loffset="-12H").mean()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right unless we want to remove the loffset kwarg.

If we keep it, then only expected needs to be changed. We'll need to update the code for loffset in resample.py to avoid the pandas warning.

If we want to remove it, I think we should remove it after indexes are more prominent and user-friendly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right.

I was thinking we'd align our API with pandas' — unless we think we should define our own API — but then we actually need to make the changes rather than remove the tests!

expected = DataArray(array.to_series().resample("24H", loffset="-12H").mean())
assert_identical(expected, actual)
expected_ = array.to_series().resample("24H").mean()
expected_.index += to_offset("-12H")
expected = DataArray.from_series(expected_)
assert_identical(actual, expected)

with raises_regex(ValueError, "index must be monotonic"):
array[[2, 0, 1]].resample(time="1D")
Expand Down
13 changes: 8 additions & 5 deletions xarray/tests/test_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import pandas as pd
import pytest
from pandas.core.indexes.datetimes import DatetimeIndex
from pandas.tseries.frequencies import to_offset

import xarray as xr
from xarray import (
Expand Down Expand Up @@ -3899,11 +3900,13 @@ def test_resample_loffset(self):
)
ds.attrs["dsmeta"] = "dsdata"

actual = ds.resample(time="24H", loffset="-12H").mean("time").time
expected = xr.DataArray(
ds.bar.to_series().resample("24H", loffset="-12H").mean()
).time
assert_identical(expected, actual)
# Our use of `loffset` may change if we align our API with pandas' changes.
# ref https://github.com/pydata/xarray/pull/4537
actual = ds.resample(time="24H", loffset="-12H").mean().bar
expected_ = ds.bar.to_series().resample("24H").mean()
expected_.index += to_offset("-12H")
expected = DataArray.from_series(expected_)
assert_identical(actual, expected)

def test_resample_by_mean_discarding_attrs(self):
times = pd.date_range("2000-01-01", freq="6H", periods=10)
Expand Down