Calendar utilities

[aulemahal]

• Closes Calendar utilities #5155
• Tests added
• Passes pre-commit run --all-files
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

So:

• Added coding.cftime_offsets.date_range and coding.cftime_offsets.date_range_like
  The first simply swtiches between pd.date_range and xarray.cftime_range according to the arguments. The second infers start, end and freq from an existing datetime array and returns a similar range in another calendar.

• Added coding/calendar_ops.py with convert_calendar and interp_calendar
  Didn't know where to put them, so there they are.

• Added DataArray.dt.calendar.
  When the datetime objects are backed by numpy, it always return "proleptic_gregorian".

I'm not sure where to expose the function. Should the range-generators be accessible directly like xr.date_range?

The convert_calendar and interp_calendar could be implemented as methods of DataArray and Dataset, should I do that?

[aulemahal]

I am excessively confused at the errors in the builds... They are triggered by tests not touching my work and also it says the installed version of xarray is 0.11???

[keewis]

where do you see that? The only thing I see is a failed docs build, which fails because you used :py:property: instead of :py:attr:, and because date_range and date_range_like are not imported by the main xarray/__init__.py

[aulemahal]

I see the strange errors here : https://github.com/aulemahal/xarray/actions. The failure triggered an email and this is why I didn't look here. Well, if it's something in the configuration of my fork, then we don't care!

Will fix the doc errors...

[keewis]

I think that's because you didn't push the newer tags, so while you get the latest commits setuptools-scm thinks the latest release is 0.11.

In any case, the action shouldn't run (it should either be skipped or not run at all). Actually, I'd recommend to disable action runs on your fork (this is possible somewhere in the repository settings) because it doesn't really make sense to run the actions multiple times.

[mathause]

I have not checked the whole PR but left some small comments anyway.

I can see that the functionalities build on each other but it would be easier to review if it was split into two or three PRs (e.g. DataArray.dt.calendar and the rest). But don't do that right now - lets see what @spencerkclark thinks.

[spencerkclark]

Thanks @aulemahal -- it will take me a few more passes to fully review this -- sorry for being a bit slow, but here are a few initial comments.

[aulemahal]

Updated the code following first wave of suggestions!

[spencerkclark]

Thanks for the nice updates @aulemahal. I made another pass and left a few more comments. Regarding your initial questions:

I'm not sure where to expose the function. Should the range-generators be accessible directly like xr.date_range?

Yes, I think this makes the most sense, considering that cftime_range is accessible from a similar place.

The convert_calendar and interp_calendar could be implemented as methods of DataArray and Dataset, should I do that?

My thought would be to say yes to this as well. Both require a DataArray or Dataset as input and resemble reindexing or interpolation operations.

[pep8speaks]

Hello @aulemahal! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-26 20:04:00 UTC

[aulemahal]

@spencerkclark Applied suggestions from the review, added Dataset.convert_calendar and Dataset.interp_calendar, and same on DataArray.

Wasn't sure what to do with the tests. They already done in test_calendar_ops.py...

[aulemahal]

@spencerkclark Done!

Note that we have this issue : Ouranosinc/xclim#841 open on our side. It's for a third method for converting 360_day calendars. Based on something used in the litterature, it randomizes the days to insert/drop when converting. My plan is to implement it in xclim soonish, then if it works I could bring it up here and open a new PR.

[spencerkclark]

Awesome, thanks @aulemahal. The tests you added for the errors look great. I have a few more suggestions for making some of the other tests a little stronger, and a question about the behavior of convert_calendar.

[aulemahal]

Here we are! Also, I don't think the test failure comes from my changes.

[spencerkclark]

OK @aulemahal, I gave this another look over today and I think this may be ready to go -- just address these last few docstring nits and merge main. I'll leave it open for the next few days in case others would like to have a look, but if I do not hear anything before the end of Wednesday, I will go ahead and merge this on Thursday morning.

[Illviljan]

Thanks @aulemahal!