-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Checks for tz/tzinfo validity in Timestamp.__new__ #17943
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
Changes from 7 commits
7914f55
5c89630
6a2f766
b4c98fb
8f304a6
f812e90
d90bc25
4839ffb
98b5b17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ from util cimport (is_integer_object, is_float_object, is_datetime64_object, | |
is_timedelta64_object, INT64_MAX) | ||
cimport util | ||
|
||
from cpython.datetime cimport PyTZInfo_Check | ||
# this is our datetime.pxd | ||
from datetime cimport ( | ||
pandas_datetimestruct, | ||
|
@@ -68,7 +69,7 @@ from .tslibs.parsing import parse_datetime_string | |
|
||
cimport cython | ||
|
||
from pandas.compat import iteritems, callable | ||
from pandas.compat import iteritems | ||
|
||
import collections | ||
import warnings | ||
|
@@ -373,12 +374,23 @@ class Timestamp(_Timestamp): | |
FutureWarning) | ||
freq = offset | ||
|
||
if tzinfo is not None: | ||
if not PyTZInfo_Check(tzinfo): | ||
# tzinfo must be a datetime.tzinfo object, GH#17690 | ||
raise TypeError('tzinfo must be a datetime.tzinfo object, ' | ||
'not %s' % type(tzinfo)) | ||
elif tz is not None: | ||
raise ValueError('Can provide at most one of tz, tzinfo') | ||
|
||
if ts_input is _no_input: | ||
# User passed keyword arguments. | ||
if tz is None: | ||
# Handle the case where the user passes `tz` and not `tzinfo` | ||
tz = tzinfo | ||
return Timestamp(datetime(year, month, day, hour or 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should set tzinfo to be None here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. |
||
minute or 0, second or 0, | ||
microsecond or 0, tzinfo), | ||
tz=tzinfo) | ||
tz=tz) | ||
elif is_integer_object(freq): | ||
# User passed positional arguments: | ||
# Timestamp(year, month, day[, hour[, minute[, second[, | ||
|
@@ -387,6 +399,10 @@ class Timestamp(_Timestamp): | |
year or 0, month or 0, day or 0, | ||
hour), tz=hour) | ||
|
||
if tzinfo is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this shouldn't be necessary if you do as the above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? Changing line 390 above doesn't affect anything at this point in the func. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you have the exact same check above (if tzinfo is not none), so you shoul do this up there. (line 377) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK. That requires conditioning on |
||
# User passed tzinfo instead of tz; avoid silently ignoring | ||
tz, tzinfo = tzinfo, None | ||
|
||
ts = convert_to_tsobject(ts_input, tz, unit, 0, 0) | ||
|
||
if ts.value == NPY_NAT: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move to 0.22.0 same section