Relax nanosecond datetime restriction in CF time decoding

[kmuehlbauer]

• Closes Interoperability with Pandas 2.0 non-nanosecond datetime #7493
• Closes DataArray constructor still coerces to np.datetime64[ns], not cftime in 0.11.0 #2587
• Tests added/changed
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

This is another attempt to resolve #7493. This goes a step further than #9580.

The idea of this PR is to automatically infer the needed resolutions for decoding/encoding and only keep the constraints pandas imposes ("s" - lowest resolution, "ns" - highest resolution). There is still the idea of a default resolution, but this should only take precedence if it doesn't clash with the automatic inference. This can be discussed, though. Update: I've implemented time-unit-kwarg a first try to have default resolution on decode, which will override the current inferred resolution only to higher resolution (eg. 's' -> 'ns'). To work towards #4490 the time decoding options (decode_time and use_cftime are bundled within CFDatetimeCoder which is distributed via decode_times kwarg. use_cftime-kwarg is deprecated.

For sanity checking, and also for my own good, I've created a documentation page on time-coding in the internal dev section. Any suggestions (especially grammar) or ideas for enhancements are much appreciated.

There still might be room for consolidation of functions/methods (mostly in coding/times.py), but I have to leave it alone for some days. I went down that rabbit hole and need to relax, too 😬.

Looking forward to get your insights here, @spencerkclark, @ChrisBarker-NOAA, @pydata/xarray.

Todo:

• floating point handling
• update decoding tests to iterate over time_units (where appropriate)
• CFDatetimeCoder as input for decode_times kwarg
• ...

[kmuehlbauer]

Nice, mypy 1.12 is out and breaks our typing, 😭.

[TomNicholas]

Nice, mypy 1.12 is out and breaks our typing, 😭

Can we pin it in the CI temporarily?

[kmuehlbauer]

Can we pin it in the CI temporarily?

Yes, 1.11.2 was the last version.

[kmuehlbauer]

This is now ready for a first round of review. I think this is already in a quite usable state.

But no rush, this should be thoroughly tested.

[spencerkclark]

Sounds good @kmuehlbauer! I’ll try and take an initial look this weekend.

[kmuehlbauer]

I tried to give it a first thorough look through. Some small suggestions, a few possible minor remaining issues, but I think the big picture is looking good!

@spencerkclark Thanks very much for the extensive review! I've addressed as much of your comments and suggestions, added todo's to keep track of the remaining.

I'll work on your suggestions in time-coding.rst the next days.

[dcherian]

Amazing stuff, Kai!

[kmuehlbauer]

@dcherian Thanks for taking another look here. I'm deep in the weeds to iron out the last remaining issues. Hope to fix this up until next week. 🤞

[kmuehlbauer]

Slowly approaching the gates...

I think I've addressed all suggestions and review comments. Good chance that I've missed something, though.

One final unresolved issue is with polyfit/integration (cc @dcherian). There seems to be some hardcoding of "ns". Unfortunately this is not my home turf, so maybe someone more experienced should take this on.

The current state of this PR should be backward compatible wrt to time decoding. There are slight changes (and also enhancements/bug fixes). All non-nano enhancements are opt-in (time_unit-kwarg to xr.coders.CFDatetimeCoder. I've left todo's in the code where to pick up in future PR's. If one of those todo's should be fixed in this PR, please have another review.

I'll need to summarize the changes in the whats-new.rst.

[shoyer]

One final unresolved issue is with polyfit/integration (cc @dcherian). There seems to be some hardcoding of "ns". Unfortunately this is not my home turf, so maybe someone more experienced should take this on.

As long as the unit tests are passing, let's open an issue for this and leave it for later. This PR is big enough already. It would be best to continue this work in smaller follow-on PRs.

My cursory examination of datetime_to_numeric() (called by interp and integrate internally) is that it should be totally with different datetime units. In general we only have to worry about extremely unsafe cases where code assumes a datetime array already has ns precision without checking.

[dcherian]

One final unresolved issue is with polyfit/integration (cc @dcherian). There seems to be some hardcoding of "ns". Unfortunately this is not my home turf, so maybe someone more experienced should take this on.

I don't think there are changes to make in polyfit per se. We should just call this out in the release notes and the docstrings. Note that the unit matters for integrate and differentiate too.

I agree with doing this in a followup.

[dcherian]

Stephan's right, we force ns when converting to numeric for polyfit/polyval so this PR isn't breaking things

xarray/xarray/core/computation.py

Line 2117 in ceb5364

data=datetime_to_numeric(x.data, offset=offset, datetime_unit="ns"),

I pushed a couple of extra tests to make sure we don't unintentionally break this in the future.

EDIT: opened #9937