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

Convert 360_day calendars by choosing random dates to drop or add #8603

Merged
merged 9 commits into from
Apr 16, 2024

Conversation

aulemahal
Copy link
Contributor

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

Small PR to add a new "method" to convert to and from 360_day calendars. The current two methods (chosen with the align_on keyword) will always remove or add the same day-of-year for all years of the same length.

This new option will randomly chose the days, one for each fifth of the year (72-days period). It emulates the method of the LOCA datasets (see web page and article ). February 29th is always removed/added when the source/target is a leap year.

I copied the implementation from xclim (which I wrote), see code here .

@aulemahal aulemahal changed the title Convert 360 calendar, choosing random dates to drop or add Convert 360_day calendars by choosing random dates to drop or add Jan 10, 2024
@aulemahal
Copy link
Contributor Author

@spencerkclark
No rush, but if you have time looking at this, this is the last piece of calendar converting that exists only in xclim. Merging this would let us get rid of duplicated code. Thanks!

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 @aulemahal—sorry for taking so long to take a look at this. The tests / implementation look reasonable.

xarray/coding/calendar_ops.py Outdated Show resolved Hide resolved
doc/whats-new.rst Outdated Show resolved Hide resolved
xarray/coding/calendar_ops.py Outdated Show resolved Hide resolved
@aulemahal
Copy link
Contributor Author

@spencerkclark I think everything here is done. Thanks!

@spencerkclark
Copy link
Member

Yup looks good—thanks!

@spencerkclark spencerkclark merged commit 239309f into pydata:main Apr 16, 2024
31 checks passed
dcherian added a commit to djhoese/xarray that referenced this pull request Apr 18, 2024
* main:
  (feat): Support for `pandas` `ExtensionArray` (pydata#8723)
  Migrate datatree mapping.py (pydata#8948)
  Add mypy to dev dependencies (pydata#8947)
  Convert 360_day calendars by choosing random dates to drop or add (pydata#8603)
@keewis keewis mentioned this pull request Apr 21, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants