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

deprecate cftime_range() in favor of date_range(use_cftime=True) #10024

Merged
merged 47 commits into from
Feb 27, 2025

Conversation

Maddogghoek
Copy link
Contributor

@Maddogghoek Maddogghoek commented Feb 4, 2025

No functional changes beyond a deprecation warning, so no test changes.

Copy link

welcome bot commented Feb 4, 2025

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.

Maddogghoek and others added 5 commits February 4, 2025 16:03
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.

Great, @Maddogghoek! Small change, big impact, thanks for tackling this!

Just one question and one suggestion. Hope this attracts more reviewers.

Maddogghoek and others added 2 commits February 14, 2025 10:14
Fix typo in (s/true/True) in what's new for issue pydata#9886 change.

Co-authored-by: Kai Mühlbauer <[email protected]>
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.

Thanks for taking this on @Maddogghoek! It took me a moment to follow the additional refactoring within _cftime_range, but generally I think those changes are incremental improvements, so I am OK to leave them in. I have a few nits and some documentation suggestions, but otherwise this is looking good.

Maddogghoek and others added 14 commits February 18, 2025 14:57
Update documentation to steer users to xarray.date_range rather than deprecated xarray.cftime_range.

Co-authored-by: Spencer Clark <[email protected]>
listed under deprecations instead

Co-authored-by: Spencer Clark <[email protected]>
Co-authored-by: Spencer Clark <[email protected]>
…ls.emit_user_level_warning rather than warnings.warn
Co-authored-by: Spencer Clark <[email protected]>
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.

Thanks @Maddogghoek! I appreciate your careful attention to detail on this. A couple more minor suggestions, but otherwise this looks good to me.


warn(
emit_user_level_warning(
Copy link
Member

Choose a reason for hiding this comment

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

Nice, thanks for updating this in the contributing guide!

Maddogghoek and others added 5 commits February 20, 2025 23:40
be more explicit about the valid combinations of start, end, periods, and freq.

Co-authored-by: Spencer Clark <[email protected]>
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.

@Maddogghoek 🎉 Only one minor nit.

@dcherian dcherian added the plan to merge Final call for comments label Feb 21, 2025
@dcherian dcherian merged commit 5ea1e81 into pydata:main Feb 27, 2025
29 of 31 checks passed
Copy link

welcome bot commented Feb 27, 2025

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

@dcherian
Copy link
Contributor

Thanks @Maddogghoek welcome to Xarray!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate xr.cftime_range in favor of xr.date_range
5 participants