DEPR: tz kwarg in Period.to_timestamp by jbrockmendel · Pull Request #34522 · 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
Conversation8 Commits4 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 }})
- closes #xxxx
- tests added / passed
- passes
black pandas - passes
git diff upstream/master -u -- "*.py" | flake8 --diff - whatsnew entry
inconsistent with PeriodArray/PeriodIndex methods
pls add this to the deprecation removal issue
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a motivation?
Is there a motivation?
As mentioned in the OP, its inconsistent with the PeriodArray/PeriodIndex methods.
Whoops, sorry, missed that sentence.
Now, this keyword doesn't seem really a burden to me, so I still don't find that a fully convincing reason (could it be added to the array method?)
Also, it seems we are using this feature internally in pandas, according to the test failure.
DEPR: tz kwarg in Period.tz_convert DEPR: tz kwarg in Period.tz_timestamp
DEPR: tz kwarg in Period.tz_timestamp DEPR: tz kwarg in Period.to_timestamp
Also, it seems we are using this feature internally in pandas, according to the test failure.
It's in timeseries.py. It can of course easily be fixed there to use tz_localize
Now, this keyword doesn't seem really a burden to me, so I still don't find that a fully convincing reason (could it be added to the array method?)
Either way works. My default is to prefer a smaller API surface, but not a strong preferenc.
Note also that Period.to_timestamp incorrectly ignores the tz arg in the how == 'end' case, so i see it as deprecating a broken feature.
agree with @jbrockmendel here; simpler is better, we always convert to naive, which is inline the model of Periods which are naive by definition.
OK, if it's only partly implemented, that's certainly another reason to deprecate it