-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
ENH: Allow Timestamp to accept Nanosecond argument #19889
Changes from all commits
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 |
---|---|---|
|
@@ -107,6 +107,7 @@ cdef class _Timestamp(datetime): | |
cdef readonly: | ||
int64_t value, nanosecond | ||
object freq # frequency reference | ||
list _date_attributes | ||
|
||
def __hash__(_Timestamp self): | ||
if self.nanosecond: | ||
|
@@ -425,6 +426,8 @@ class Timestamp(_Timestamp): | |
.. versionadded:: 0.19.0 | ||
hour, minute, second, microsecond : int, optional, default 0 | ||
.. versionadded:: 0.19.0 | ||
nanosecond : int, optional, default 0 | ||
.. versionadded:: 0.23.0 | ||
tzinfo : datetime.tzinfo, optional, default None | ||
.. versionadded:: 0.19.0 | ||
|
||
|
@@ -556,7 +559,7 @@ class Timestamp(_Timestamp): | |
object freq=None, tz=None, unit=None, | ||
year=None, month=None, day=None, | ||
hour=None, minute=None, second=None, microsecond=None, | ||
tzinfo=None): | ||
nanosecond=None, tzinfo=None): | ||
# The parameter list folds together legacy parameter names (the first | ||
# four) and positional and keyword parameter names from pydatetime. | ||
# | ||
|
@@ -580,6 +583,9 @@ class Timestamp(_Timestamp): | |
|
||
cdef _TSObject ts | ||
|
||
_date_attributes = [year, month, day, hour, minute, second, | ||
microsecond, nanosecond] | ||
|
||
if tzinfo is not None: | ||
if not PyTZInfo_Check(tzinfo): | ||
# tzinfo must be a datetime.tzinfo object, GH#17690 | ||
|
@@ -588,28 +594,35 @@ class Timestamp(_Timestamp): | |
elif tz is not None: | ||
raise ValueError('Can provide at most one of tz, tzinfo') | ||
|
||
if ts_input is _no_input: | ||
if is_string_object(ts_input): | ||
# User passed a date string to parse. | ||
# Check that the user didn't also pass a date attribute kwarg. | ||
if any(arg is not None for arg in _date_attributes): | ||
raise ValueError('Cannot pass a date attribute keyword ' | ||
'argument when passing a date string') | ||
|
||
elif 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, | ||
minute or 0, second or 0, | ||
microsecond or 0, tzinfo), | ||
tz=tz) | ||
nanosecond=nanosecond, tz=tz) | ||
elif is_integer_object(freq): | ||
# User passed positional arguments: | ||
# Timestamp(year, month, day[, hour[, minute[, second[, | ||
# microsecond[, tzinfo]]]]]) | ||
# microsecond[, nanosecond[, tzinfo]]]]]]) | ||
return Timestamp(datetime(ts_input, freq, tz, unit or 0, | ||
year or 0, month or 0, day or 0, | ||
hour), tz=hour) | ||
minute), nanosecond=hour, tz=minute) | ||
|
||
if tzinfo is not None: | ||
# User passed tzinfo instead of tz; avoid silently ignoring | ||
tz, tzinfo = tzinfo, None | ||
|
||
ts = convert_to_tsobject(ts_input, tz, unit, 0, 0) | ||
ts = convert_to_tsobject(ts_input, tz, unit, 0, 0, nanosecond 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. What if we get here with a string ts_input with conflicting nanos? I think it might be better to accept nanos only in combination with other integer args/kwargs. 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. Currently (and in this PR), the Timestamp constructor will just parse the string and ignore all additional passed kwargs related to the values of each datetime element:
The nanosecond kwarg will only be used if a datetime.datetime is passed or other datetime element kwargs are passed. As mentioned in my other comment, we might also want to raise in the situation 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. I agree here, the nanoscecond kw must only be used when kwargs are passed |
||
|
||
if ts.value == NPY_NAT: | ||
return NaT | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -385,6 +385,27 @@ def test_constructor_fromordinal(self): | |
ts = Timestamp.fromordinal(dt_tz.toordinal(), tz='US/Eastern') | ||
assert ts.to_pydatetime() == dt_tz | ||
|
||
@pytest.mark.parametrize('result', [ | ||
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. test the string case as you have a above, which should raise a ValueError |
||
Timestamp(datetime(2000, 1, 2, 3, 4, 5, 6), nanosecond=1), | ||
Timestamp(year=2000, month=1, day=2, hour=3, minute=4, second=5, | ||
microsecond=6, nanosecond=1), | ||
Timestamp(year=2000, month=1, day=2, hour=3, minute=4, second=5, | ||
microsecond=6, nanosecond=1, tz='UTC'), | ||
Timestamp(2000, 1, 2, 3, 4, 5, 6, 1, None), | ||
Timestamp(2000, 1, 2, 3, 4, 5, 6, 1, pytz.UTC)]) | ||
def test_constructor_nanosecond(self, result): | ||
# GH 18898 | ||
expected = Timestamp(datetime(2000, 1, 2, 3, 4, 5, 6), tz=result.tz) | ||
expected = expected + Timedelta(nanoseconds=1) | ||
assert result == expected | ||
|
||
@pytest.mark.parametrize('arg', ['year', 'month', 'day', 'hour', 'minute', | ||
'second', 'microsecond', 'nanosecond']) | ||
def test_invalid_date_kwarg_with_string_input(self, arg): | ||
kwarg = {arg: 1} | ||
with pytest.raises(ValueError): | ||
Timestamp('2010-10-10 12:59:59.999999999', **kwarg) | ||
|
||
def test_out_of_bounds_value(self): | ||
one_us = np.timedelta64(1).astype('timedelta64[us]') | ||
|
||
|
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.
see above