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

Jump to bottom

Support additional dtypes in resample #9413

Merged
merged 18 commits into from
Sep 7, 2024
Merged

Support additional dtypes in resample #9413

merged 18 commits into from
Sep 7, 2024

Conversation

oliverhiggs
Copy link
Contributor

pandas.BaseOffset, pandas.Timedelta, datetime.timedelta, and BaseCFTimeOffset are now all supported datatypes for resampling.

Closes #9408

@welcome Welcome
Copy link

welcome bot commented Aug 29, 2024

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

@oliverhiggs oliverhiggs mentioned this pull request Aug 29, 2024
Closed
dcherian
dcherian reviewed Aug 29, 2024
View reviewed changes
xarray/groupers.py Outdated Show resolved Hide resolved
dcherian
dcherian reviewed Aug 29, 2024
View reviewed changes
xarray/core/common.py Outdated Show resolved Hide resolved
@max-sixty
Copy link
Collaborator

Looks good! Can we add a test?

@oliverhiggs
Support additional dtypes to resample
51185ac 
pandas.BaseOffset, pandas.Timedelta, datetime.timedelta, and BaseCFTimeOffset are now all supported datatypes for resampling.
@oliverhiggs
Update whats-new
d636784
@oliverhiggs
Fix types
0d37fdd
@oliverhiggs
Add unit test
6801b79
@oliverhiggs
Fix test
36b8a63
dcherian
dcherian reviewed Sep 3, 2024
View reviewed changes
xarray/groupers.py Outdated Show resolved Hide resolved
@oliverhiggs
Copy link
Contributor Author

This is failing a test on the bare-minimum environment because of a FutureWarning caused by pandas 2.0.3 using 'S' instead of 's' for an internal frequency string. I noticed there are other FutureWarnings that occur during the test run but don't cause a test failure, why is this? Is it possible to do the same with my test?

e.g. This warning does not cause a test failure.

  /home/runner/work/xarray/xarray/xarray/tests/test_variable.py:2421: FutureWarning: the `pandas.MultiIndex` object(s) passed as 'x' coordinate(s) or data variable(s) will no longer be implicitly promoted and wrapped into multiple indexed coordinates in the future (i.e., one coordinate for each multi-index level + one dimension coordinate). If you want to keep this behavior, you need to first wrap it explicitly using `mindex_coords = xarray.Coordinates.from_pandas_multiindex(mindex_obj, 'dim')` and pass it as coordinates, e.g., `xarray.Dataset(coords=mindex_coords)`, `dataset.assign_coords(mindex_coords)` or `dataarray.assign_coords(mindex_coords)`.
    ds = Dataset(coords={"x": midx})

dcherian
dcherian reviewed Sep 5, 2024
View reviewed changes
doc/whats-new.rst Outdated Show resolved Hide resolved
@dcherian
Copy link
Contributor

dcherian commented Sep 5, 2024 •
edited
Loading

yes we can take it from here. Thanks for your contribution and welcome to Xarray!

@dcherian dcherian added the plan to merge Final call for comments label Sep 5, 2024
@oliverhiggs
Copy link
Contributor Author

Thanks! And thanks for creating such a welcoming atmosphere for new contributors.

spencerkclark
spencerkclark approved these changes Sep 7, 2024
View reviewed changes
Copy link
Member

@spencerkclark spencerkclark left a comment

Choose a reason for hiding this comment

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

@oliverhiggs sorry for the delay—this looks great!

To answer your question about the warning leading to a test failure—a few months ago we made a change to make any new warnings emitted in the test suite errors (#8974). In that PR some code was added to ignore some existing warnings (the MultiIndex warning you noted being one of them).

We've dealt with the frequency string deprecation warning in the past in the context of old pandas versions (#8627); I made a suggestion which I think may resolve it.

xarray/coding/cftime_offsets.py Outdated Show resolved Hide resolved
xarray/coding/cftime_offsets.py Outdated Show resolved Hide resolved
dcherian and others added 2 commits September 7, 2024 07:17
@dcherian @spencerkclark
Apply suggestions from code review
7649cb4 
Co-authored-by: Spencer Clark <spencerkclark@gmail.com>
@spencerkclark
Fix mypy error
50b69d9
@dcherian dcherian merged commit a74a605 into pydata:main Sep 7, 2024
28 checks passed
@welcome Welcome
Copy link

welcome bot commented Sep 7, 2024

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! celebration gif

dcherian added a commit to dcherian/xarray that referenced this pull request Sep 17, 2024
@dcherian
Merge branch 'main' into groupby-shuffle
0679d2b 
* main: (29 commits)
  Release notes for v2024.09.0 (pydata#9480)
  Fix `DataTree.coords.__setitem__` by adding `DataTreeCoordinates` class (pydata#9451)
  Rename DataTree's "ds" and "data" to "dataset" (pydata#9476)
  Update DataTree repr to indicate inheritance (pydata#9470)
  Bump pypa/gh-action-pypi-publish in the actions group (pydata#9460)
  Repo checker (pydata#9450)
  Add days_in_year and decimal_year to dt accessor (pydata#9105)
  remove parent argument from DataTree.__init__ (pydata#9465)
  Fix inheritance in DataTree.copy() (pydata#9457)
  Implement `DataTree.__delitem__` (pydata#9453)
  Add ASV for datatree.from_dict (pydata#9459)
  Make the first argument in DataTree.from_dict positional only (pydata#9446)
  Fix typos across the code, doc and comments (pydata#9443)
  DataTree should not be "Generic" (pydata#9445)
  Disallow passing a DataArray as data into the DataTree constructor (pydata#9444)
  Support additional dtypes in `resample` (pydata#9413)
  Shallow copy parent and children in DataTree constructor (pydata#9297)
  Bump minimum versions for dependencies (pydata#9434)
  Always include at least one category in random test data (pydata#9436)
  Avoid deep-copy when constructing groupby codes (pydata#9429)
  ...
hollymandel pushed a commit to hollymandel/xarray that referenced this pull request Sep 23, 2024
@oliverhiggs @pre-commit-ci
@dcherian @spencerkclark @hollymandel
Support additional dtypes in resample (pydata#9413)
eda216d 
* Support additional dtypes to resample

pandas.BaseOffset, pandas.Timedelta, datetime.timedelta, and BaseCFTimeOffset are now all supported datatypes for resampling.

* Update whats-new

* Fix types

* Add unit test

* Fix test

* Support more dtypes for CFTimeIndex resampling

* Tidy resample type hints

* Fix some mypy bugs

* Fixes

* Fix tests

* WIP

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update doc/whats-new.rst

* Apply suggestions from code review

Co-authored-by: Spencer Clark <spencerkclark@gmail.com>

* Fix mypy error

* Fix bad edit

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
Co-authored-by: Spencer Clark <spencerkclark@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spencerkclark spencerkclark spencerkclark approved these changes

@dcherian dcherian dcherian approved these changes

Assignees
No one assigned
Labels
plan to merge Final call for comments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Resample no longer works with Pandas DateOffset or Timedelta objects
4 participants
@oliverhiggs @max-sixty @dcherian @spencerkclark