DEPR: deprecate default of keep_tz=False of DatetimeIndex.to_series #23739
Conversation
jorisvandenbossche
added
Datetime
|
Hello @jorisvandenbossche! Thanks for submitting the PR.
|
| @@ -500,6 +500,12 @@ def to_series(self, keep_tz=False, index=None, name=None): | |||
|
|
|||
| Series will have a datetime64[ns] dtype. TZ aware | |||
| objects will have the tz removed. | |||
|
|
|||
| .. versionchanged:: 0.24 | |||
| The default value will change to True in a future release. | |||
There was a problem hiding this comment.
I would just remove the keyword in the future, we don't need to offer this additional funtionaility, this was mainly for backwards compat originally.
There was a problem hiding this comment.
Yes, my idea is to remove it in the future. But I think we first need to deprecate the behaviour, so people can switch, and once that is done, we can deprecate the full keyword.
See also some of the notes about it in #17832
There was a problem hiding this comment.
But, we can of course already discourage the 'False' behaviour, by also deprecating that (only allowing keep_tz=True
There was a problem hiding this comment.
yeah I think i would deprecate any non-None
| with tm.assert_produces_warning(FutureWarning): | ||
| result = idx.to_series(index=[0, 1]) | ||
| tm.assert_series_equal( | ||
| result, expected.dt.tz_convert('UTC').dt.tz_localize(None)) |
There was a problem hiding this comment.
This might seem a strange file to put this test, but as far as I found, this is actually the only place where this behaviour was tested. So I decided to put it here, so only one test needs to be updated when we remove the deprecation.
|
lgtm. |
|
can you add to the deprecation issue #6581 |
Codecov Report
@@ Coverage Diff @@
## master #23739 +/- ##
==========================================
+ Coverage 92.25% 92.25% +<.01%
==========================================
Files 161 161
Lines 51383 51388 +5
==========================================
+ Hits 47404 47409 +5
Misses 3979 3979
Continue to review full report at Codecov.
|
|
thanks @jorisvandenbossche |
|
I'm looking at enforcing this deprecation now. Should we just change the default or go ahead and rip out keep_tz altogether? |
Closes #17832