Silence datetime accessor deprecation warnings#11270
Silence datetime accessor deprecation warnings#11270spencerkclark wants to merge 8 commits intopydata:mainfrom
Conversation
b1580db to
3d4c471
Compare
f833723 to
d676fbf
Compare
37dc249 to
16c0b3a
Compare
|
Do we really need to deprecate these? They've been around for so long, perhaps we should just keep both names. |
|
Yeah, I don't have a particularly strong opinion. I'm fine with maintaining the other aliases. I'll revert those changes when I get a chance. |
db9fad4 to
30eb968
Compare
30eb968 to
bd99e04
Compare
|
|
||
| weekday = dayofweek | ||
| dayofweek = day_of_week | ||
| weekday = day_of_week |
There was a problem hiding this comment.
An awkward aspect of this approach for adding datetime accessor aliases, which was present before this PR, is that the name of the resulting DataArray is inconsistent with the name of the alias, e.g. in the current version of xarray:
>>> times = xr.date_range("2000", periods=10)
>>> xr.DataArray(times, dims=["time"]).dt.weekday
<xarray.DataArray 'dayofweek' (time: 10)> Size: 80B
array([5, 6, 0, 1, 2, 3, 4, 5, 6, 0])
Coordinates:
* time (time) datetime64[ns] 80B 2000-01-01 2000-01-02 ... 2000-01-10
A current consequence of this in this PR is that the name of these DataArrays will change:
>>> xr.DataArray(times, dims=["time"]).dt.weekday
<xarray.DataArray 'day_of_week' (time: 10)> Size: 80B
array([5, 6, 0, 1, 2, 3, 4, 5, 6, 0])
Coordinates:
* time (time) datetime64[ns] 80B 2000-01-01 2000-01-02 ... 2000-01-10
We could rename to always match the old names if we want to be as conservative as possible, but it feels more intuitive for the name of the DataArray to match the name of the accessor attribute. Should we address this as part of this PR as well?
There was a problem hiding this comment.
Since these properties are just one line of code, it doesn't hurt to add the aliases as "real" properties back. This also allows us to depreciate it as well (do we want to?).
There was a problem hiding this comment.
Yes, my second suggestion would involve defining the aliases as their own independent properties to give us control over the name of the resulting DataArrays. My feeling is that we could consider the name mismatch a "bug" which we could fix without a deprecation cycle, but I am open to opinions from others.
7067a40 to
7039d7b
Compare
spencerkclark
left a comment
There was a problem hiding this comment.
I went ahead and addressed #11270 (comment) by ensuring the names of the DataArrays returned by the DatetimeAccessor always match the names of the properties.
I'm happy to go with the more conservative approach if anyone feels strongly, but it felt a bit silly to go out of our way to maintain the previous names when they were already somewhat counterintuitive.
| warnings.warn( | ||
| "dt.weekofyear and dt.week have been deprecated. Please use " | ||
| "dt.isocalendar().week instead.", | ||
| FutureWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
We could take the liberty of finalizing the deprecation cycle for these weekofyear and week properties or simply un-deprecate them.
This PR silences datetime-related warnings surfaced in the latest upstream build. It also follows pandas in deprecating redundant accessor names, and adds
day_of_weekandday_of_yearaccessors toCFTimeIndex.np.timedelta64dtype #11281, this will close ⚠️ Nightly upstream-dev CI failed ⚠️ #11268whats-new.rst