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.

Sign up for GitHub

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

Adjust tests to use updated pandas syntax for offsets #4537

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9399ed4
Adjust tests to use updated pandas syntax for offsets
max-sixty Oct 24, 2020
d25e983
Revert irrelevant change
max-sixty Oct 25, 2020
bbdd764
Retain calcs for `actual`
max-sixty Nov 19, 2020
64f795a
Merge branch 'master' into loffset
max-sixty Dec 30, 2020
2155049
Merge branch 'master' into loffset
max-sixty Jan 12, 2021
376fb74
Update xarray/tests/test_dataarray.py
max-sixty Jan 12, 2021
eba911f
Apply suggestions from code review
keewis Feb 27, 2021
d920e40
fix imports
keewis Feb 27, 2021
dd0323a
Merge branch 'master' into loffset
keewis Feb 27, 2021
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
8 changes: 6 additions & 2 deletions xarray/tests/test_dataarray.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import numpy as np
import pandas as pd
import pytest
from pandas.testing import assert_index_equal
from pandas.tseries.frequencies import to_offset

import xarray as xr
from xarray import (
Expand Down Expand Up @@ -2961,9 +2963,11 @@ 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().index + to_offset("-12H")
assert_index_equal(actual, expected)
max-sixty marked this conversation as resolved.
Show resolved Hide resolved

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

import xarray as xr
from xarray import (
Expand Down Expand Up @@ -3826,11 +3828,11 @@ 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()
expected = ds.bar.to_series().resample("24H").mean().index + to_offset("-12H")
assert_index_equal(actual, expected)
keewis marked this conversation as resolved.
Show resolved Hide resolved

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