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

API: tz_convert within DatetimeIndex constructor #23579

Closed
jbrockmendel opened this issue Nov 8, 2018 · 4 comments

Comments

4 participants
@jbrockmendel
Copy link
Member

commented Nov 8, 2018

At the moment the following raises:

>>> dti = pd.date_range('2016-01-01', periods=3, tz='US/Central')
>>> pd.DatetimeIndex(dti, tz='Asia/Tokyo')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/site-packages/pandas/core/indexes/datetimes.py", line 413, in __new__
    raise TypeError(msg.format(data.tz, tz))
TypeError: data is already tz-aware US/Central, unable to set specified tz: Asia/Tokyo

It isn't clear to me that raising is the right thing to do; shouldn't this just be equivalent to dti.tz_convert('Asia/Tokyo')? Or is this ambiguous for some reason?

@mroeschke

This comment has been minimized.

Copy link
Member

commented Nov 8, 2018

This works for Timestamp albeit I am not really a fan of tz= meaning localizing and converting. But if this is properly documented, we might as well follow Timestamp's behavior unless I am missing something

In [1]: pd.Timestamp(pd.Timestamp('2016-01-01', tz='US/Central'), tz='Asia/Tokyo')
Out[1]: Timestamp('2016-01-01 15:00:00+0900', tz='Asia/Tokyo')
@jreback

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

I would rather leave this and be very explicit
doing anything non explicit with tz localize vs conversion has bitten lots in the past

@mroeschke

This comment has been minimized.

Copy link
Member

commented Nov 8, 2018

For consistency sake then, we should depreciate the Timestamp behavior then.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Nov 8, 2018

yep that sounds right

@gfyoung gfyoung added the Timeseries label Nov 11, 2018

@jreback jreback added this to the 0.24.0 milestone Nov 11, 2018

@jbrockmendel jbrockmendel added this to DTI/DTA Constructor Issues in DatetimeArray Refactor Nov 16, 2018

@jbrockmendel jbrockmendel moved this from DTI/DTA Constructor Issues to Done in DatetimeArray Refactor Dec 7, 2018

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.