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

Add time_unit argument to CFTimeIndex.to_datetimeindex #9965

Merged
merged 13 commits into from
Jan 29, 2025
Merged

Add time_unit argument to CFTimeIndex.to_datetimeindex #9965

merged 13 commits into from
Jan 29, 2025
+109 −51

Conversation

spencerkclark
Copy link
Member

Per #9618 (comment), this PR adds a time_unit keyword argument to CFTimeIndex.to_datetimeindex. The default is None, which currently means the dates will be converted to nanosecond-resolution np.datetime64 values, but a FutureWarning is raised indicating that this will eventually be switched to return microsecond-precision np.datetime64 values.

cc: @dcherian @kmuehlbauer

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

@spencerkclark spencerkclark force-pushed the to_datetimeindex-update branch 4 times, most recently from 1732fa7 to aad1c76 Compare January 20, 2025 19:05
@spencerkclark spencerkclark force-pushed the to_datetimeindex-update branch from f421467 to 6e49c2a Compare January 20, 2025 19:17
kmuehlbauer
kmuehlbauer approved these changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@kmuehlbauer kmuehlbauer left a comment

Choose a reason for hiding this comment

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

Thanks so much for taking care of the CFTimeIndex changes. It looks really complete 🚀. Just some more minor typing which could be added.

xarray/tests/test_cftime_offsets.py Outdated Show resolved Hide resolved
xarray/tests/test_cftimeindex.py Outdated Show resolved Hide resolved
xarray/tests/test_groupby.py Show resolved Hide resolved
xarray/tests/test_missing.py Outdated Show resolved Hide resolved
spencerkclark and others added 4 commits January 21, 2025 21:29
@spencerkclark @kmuehlbauer
Update xarray/tests/test_cftime_offsets.py
09c0ae4 
Co-authored-by: Kai Mühlbauer <kmuehlbauer@wradlib.org>
@spencerkclark @kmuehlbauer
Apply suggestions from code review
06e7c6a 
Co-authored-by: Kai Mühlbauer <kmuehlbauer@wradlib.org>
@spencerkclark @kmuehlbauer
Update xarray/tests/test_cftimeindex.py
04f138f 
Co-authored-by: Kai Mühlbauer <kmuehlbauer@wradlib.org>
@spencerkclark
Merge branch 'main' into to_datetimeindex-update
9ba6f75
spencerkclark and others added 2 commits January 27, 2025 20:48
@spencerkclark @dcherian
Apply Deepak's wording suggestion
1375034 
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
@spencerkclark
Merge branch 'main' into to_datetimeindex-update
5da86e0
@spencerkclark spencerkclark added the plan to merge Final call for comments label Jan 28, 2025
@kmuehlbauer
Copy link
Contributor

Is the mypy error related, as it doesn't show up in other CI runs?

@kmuehlbauer
Copy link
Contributor

Is the mypy error related, as it doesn't show up in other CI runs?

I was too eager suggestion typing enhancements. I've removed the erroneous typing for the internal function for now.

@kmuehlbauer
Copy link
Contributor

Ok, let's move on here! Thanks @spencerkclark!

@kmuehlbauer kmuehlbauer merged commit e432479 into pydata:main Jan 29, 2025
29 checks passed
@spencerkclark spencerkclark deleted the to_datetimeindex-update branch January 29, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmuehlbauer kmuehlbauer kmuehlbauer approved these changes

@dcherian dcherian dcherian approved these changes

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

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