Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEPR: deprecate keep_tz keyword in DatetimeIndex.to_series #17832

Closed
jorisvandenbossche opened this issue Oct 10, 2017 · 2 comments

Comments

Projects
None yet
3 participants
@jorisvandenbossche
Copy link
Member

commented Oct 10, 2017

xref #17826

DatetimeIndex.to_series has a keep_tz keyword with a default of False (= dropping the timezone information). This stems from a time we could not store datetime tz data inside a series (with keep_tz=True, you would get an object dtyped Series of Timestamps). Nowadays this just gives a series with datetime64[ns, tz] dtype, so it makes sense to make this the default and deprecate the keyword.

Since it is an API change, ideally we first raise a warning that the behaviour will be changed (the default will change from False to True). You could then suppress this warning by passing keep_tz=True.
However, that means that we cannot directly deprecate the keyword itself, as we have to keep it for suppressing the warning / getting the future behaviour.

@gfyoung

This comment has been minimized.

Copy link
Member

commented Oct 10, 2017

Indeed, how we approach this will depend on when the release gets cut. If we get this in before the cut, we could easily just change the default behavior + deprecate in one PR. Otherwise, we would have to split it into two across multiple releases.

@jorisvandenbossche

This comment has been minimized.

Copy link
Member Author

commented Oct 10, 2017

Why does this depend on when the rc gets cut? (when I mentioned it, I was referring to whether I would have time to do it before the rc gets cut)

How can we change the behaviour and deprecate in one PR? You mean just changing the behaviour without warning? Which is actually certainly an option, as this prevents having to do the deprecation in two steps (first behaviour, then keyword), and I don't think this is that much used, so I am OK with just a back incompat breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.