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

ERR: Timestamp.__new__ accepts both tz and tzinfo #17690

Closed
jbrockmendel opened this Issue Sep 27, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@jbrockmendel
Copy link
Member

commented Sep 27, 2017

Why are there separate tz and tzinfo kwargs? The following is not intuitive:

In [12]: pd.Timestamp('2017-09-25 21:23', tz='US/Eastern')
Out[12]: Timestamp('2017-09-25 21:23:00-0400', tz='US/Eastern')

In [13]: pd.Timestamp('2017-09-25 21:23', tzinfo='US/Eastern')
Out[13]: Timestamp('2017-09-25 21:23:00')
@jreback

This comment has been minimized.

Copy link
Contributor

commented Sep 27, 2017

tzinfo is the kw that datetime.datetime accepts. tz is much more inuitive (as its a string).

[13] tzinfo is invalid (so maybe should check this).

@jreback jreback changed the title Timestamp.__new__ accepts both tz and tzinfo ERR: Timestamp.__new__ accepts both tz and tzinfo Sep 27, 2017

@jreback jreback added this to the Next Major Release milestone Sep 27, 2017

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2017

I'd be OK with a "well the user clearly intended..." approach or a raising approach, but don't like silently ignoring the input.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Sep 28, 2017

yeah this need some internal conversion for tzinfo; this is already well handled for tz (which is aliased but not treated exactly the same).

some tests should nail this down.

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2017

Any preference between the raise for accept-when-unambiguous? I lean slightly towards the latter.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Sep 30, 2017

tzinfo as a string is plain wrong and should raise

jbrockmendel added a commit to jbrockmendel/pandas that referenced this issue Oct 22, 2017

@jreback jreback modified the milestones: Next Major Release, 0.22.0 Oct 27, 2017

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.