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

Enable origin and offset arguments in resample #7284

Merged
merged 21 commits into from
Nov 28, 2022
Merged

Enable origin and offset arguments in resample #7284

merged 21 commits into from
Nov 28, 2022

Conversation

spencerkclark
Copy link
Member

@spencerkclark spencerkclark commented Nov 13, 2022 •
edited
Loading

This PR enables the origin and offset arguments in resample. This was simple to do in the case of data indexed by a DatetimeIndex, but naturally required changes to our internal implementation of resample for data indexed by a CFTimeIndex. Fortunately those changes were fairly straightforward to port over from pandas.

This does not do anything to address the deprecation of base noted in #7266, but is an important first step toward getting up to speed with the latest version of pandas, both on the DatetimeIndex side and the CFTimeIndex side. This way we will at least be able to handle that deprecation in the same way for each.

  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

I think things are fairly comprehensively implemented and tested here, but I'm marking this as a draft for now as I want to see if I can reduce the number of cftime resampling tests some, which have multiplied with the addition of these new arguments.

@spencerkclark spencerkclark mentioned this pull request Nov 13, 2022
Closed
headtr1ck
headtr1ck approved these changes Nov 16, 2022
View reviewed changes
Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

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

In general there are a lot of ifs but I don't see any other way.

If you are still up for it, I would appreciate it if you could type annotate your added functions.

xarray/core/common.py Show resolved Hide resolved
xarray/core/resample_cftime.py Outdated Show resolved Hide resolved
xarray/core/resample_cftime.py Outdated Show resolved Hide resolved
xarray/tests/test_cftimeindex_resample.py Show resolved Hide resolved
@spencerkclark spencerkclark marked this pull request as ready for review November 25, 2022 21:21
headtr1ck
headtr1ck reviewed Nov 25, 2022
View reviewed changes
xarray/core/types.py Outdated Show resolved Hide resolved
@headtr1ck headtr1ck added the plan to merge Final call for comments label Nov 27, 2022
@dcherian dcherian enabled auto-merge (squash) November 28, 2022 23:18
@dcherian dcherian merged commit 1083c9d into pydata:main Nov 28, 2022
@spencerkclark spencerkclark deleted the resample-update branch November 29, 2022 00:06
@spencerkclark spencerkclark mentioned this pull request Nov 29, 2022
Merged
headtr1ck pushed a commit to headtr1ck/xarray that referenced this pull request Nov 29, 2022
@spencerkclark @pre-commit-ci
@dcherian @headtr1ck
Enable origin and offset arguments in resample (pydata#7284)
91fda37 
* Initial work toward enabling origin and offset arguments in resample

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

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

* Fix _convert_offset_to_timedelta

* Reduce number of tests

* Address initial review comments

* Add more typing information

* Make cftime import lazy

* Fix module_available import and test

* Remove old origin argument

* Add type annotations for resample_cftime.py

* Add None as a possibility for closed and label

* Add what's new entry

* Add missing type annotation

* Delete added line

* Fix typing errors

* Add comment and test for as_timedelta stub

* Remove old code

* [test-upstream]

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak Cherian <[email protected]>
dcherian added a commit that referenced this pull request Nov 30, 2022
@headtr1ck @jhamman
@mathause @spencerkclark @pre-commit-ci @dcherian
Add parse_dims func (#7051)
e4fe194 
* add parse_dims func

* add some more tests for tuples

* add parse_ordered_dims

* fix typing issue

* remove double ellipsis typehints

* fix gen aggrs

* remove more double ellipsis typehints

* fix doctests: supress urllib3 warning (#7326)

* Enable `origin` and `offset` arguments in `resample` (#7284)

* Initial work toward enabling origin and offset arguments in resample

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

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

* Fix _convert_offset_to_timedelta

* Reduce number of tests

* Address initial review comments

* Add more typing information

* Make cftime import lazy

* Fix module_available import and test

* Remove old origin argument

* Add type annotations for resample_cftime.py

* Add None as a possibility for closed and label

* Add what's new entry

* Add missing type annotation

* Delete added line

* Fix typing errors

* Add comment and test for as_timedelta stub

* Remove old code

* [test-upstream]

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak Cherian <[email protected]>

* Fix PR number in what’s new (#7331)

* [pre-commit.ci] pre-commit autoupdate (#7330)

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.3.0 → v4.4.0](pre-commit/pre-commit-hooks@v4.3.0...v4.4.0)
- [github.com/PyCQA/autoflake: v1.7.7 → v2.0.0](PyCQA/autoflake@v1.7.7...v2.0.0)
- [github.com/PyCQA/flake8: 5.0.4 → 6.0.0](PyCQA/flake8@5.0.4...6.0.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Mathias Hauser <[email protected]>

* add comment explaining type: ignore

* fix doctest win/linux issue once again

Co-authored-by: Joe Hamman <[email protected]>
Co-authored-by: Mathias Hauser <[email protected]>
Co-authored-by: Spencer Clark <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak Cherian <[email protected]>
dcherian added a commit to dcherian/xarray that referenced this pull request Dec 2, 2022
@dcherian
Merge remote-tracking branch 'upstream/main' into groupby-save-codes-new
004a674 
* upstream/main: (39 commits)
  Support the new compression argument in netCDF4 > 1.6.0 (pydata#6981)
  Remove setuptools-scm-git-archive, require setuptools-scm>=7 (pydata#7253)
  Fix mypy failures (pydata#7343)
  Docs: add example of writing and reading groups to netcdf (pydata#7338)
  Reset file pointer to 0 when reading file stream (pydata#7304)
  Enable mypy warn unused ignores (pydata#7335)
  Optimize some copying (pydata#7209)
  Add parse_dims func (pydata#7051)
  Fix coordinate attr handling in `xr.where(..., keep_attrs=True)` (pydata#7229)
  Remove code used to support h5py<2.10.0 (pydata#7334)
  [pre-commit.ci] pre-commit autoupdate (pydata#7330)
  Fix PR number in what’s new (pydata#7331)
  Enable `origin` and `offset` arguments in `resample` (pydata#7284)
  fix doctests: supress urllib3 warning (pydata#7326)
  fix flake8 config (pydata#7321)
  implement Zarr v3 spec support (pydata#6475)
  Fix polyval overloads (pydata#7315)
  deprecate pynio backend (pydata#7301)
  mypy - Remove some ignored packages and modules (pydata#7319)
  Switch to T_DataArray in .coords (pydata#7285)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@headtr1ck headtr1ck headtr1ck approved these changes

Assignees
No one assigned
Labels
plan to merge Final call for comments topic-cftime topic-pandas-like topic-typing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spencerkclark @headtr1ck @dcherian