DEPR: deprecate default of keep_tz=False of DatetimeIndex.to_series by jorisvandenbossche · Pull Request #23739 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
| @@ -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. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But, we can of course already discourage the 'False' behaviour, by also deprecating that (only allowing keep_tz=True
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
can you add to the deprecation issue #6581
tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request
I'm looking at enforcing this deprecation now. Should we just change the default or go ahead and rip out keep_tz altogether?