Skip to content

CLN: remove pydt_to_i8 #30854

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jan 24, 2020
Merged

CLN: remove pydt_to_i8 #30854

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b84def2
CLN: remove pydt_to_i8
jbrockmendel Jan 9, 2020
e2db91d
Merge branch 'master' of https://github.com/pandas-dev/pandas into cl…
jbrockmendel Jan 10, 2020
b2a135a
explicitly cast to int64
jbrockmendel Jan 10, 2020
84f38c5
Merge branch 'master' of https://github.com/pandas-dev/pandas into cl…
jbrockmendel Jan 16, 2020
57a311b
Merge branch 'master' of https://github.com/pandas-dev/pandas into cl…
jbrockmendel Jan 16, 2020
68bf4ae
Merge branch 'master' of https://github.com/pandas-dev/pandas into cl…
jbrockmendel Jan 17, 2020
fa99d6e
Merge branch 'master' of https://github.com/pandas-dev/pandas into cl…
jbrockmendel Jan 19, 2020
b7128fc
use convert_datetime_to_tsobject
jbrockmendel Jan 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pandas/_libs/tslibs/conversion.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,4 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz,

cdef int64_t get_datetime64_nanos(object val) except? -1

cpdef int64_t pydt_to_i8(object pydt) except? -1

cpdef datetime localize_pydatetime(datetime dt, object tz)
21 changes: 0 additions & 21 deletions pandas/_libs/tslibs/conversion.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,27 +223,6 @@ cdef class _TSObject:
return self.value


cpdef int64_t pydt_to_i8(object pydt) except? -1:
"""
Convert to int64 representation compatible with numpy datetime64; converts
to UTC

Parameters
----------
pydt : object

Returns
-------
i8value : np.int64
"""
cdef:
_TSObject ts

ts = convert_to_tsobject(pydt, None, None, 0, 0)

return ts.value


cdef convert_to_tsobject(object ts, object tz, object unit,
bint dayfirst, bint yearfirst, int32_t nanos=0):
"""
Expand Down
4 changes: 2 additions & 2 deletions pandas/_libs/tslibs/offsets.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ from pandas._libs.tslibs.util cimport is_integer_object

from pandas._libs.tslibs.ccalendar import MONTHS, DAYS
from pandas._libs.tslibs.ccalendar cimport get_days_in_month, dayofweek
from pandas._libs.tslibs.conversion cimport pydt_to_i8, localize_pydatetime
from pandas._libs.tslibs.conversion cimport localize_pydatetime
from pandas._libs.tslibs.nattype cimport NPY_NAT
from pandas._libs.tslibs.np_datetime cimport (
npy_datetimestruct, dtstruct_to_dt64, dt64_to_dtstruct)
Expand Down Expand Up @@ -233,7 +233,7 @@ def _to_dt64D(dt):
# numpy.datetime64('2013-05-01T02:00:00.000000+0200')
# Thus astype is needed to cast datetime to datetime64[D]
if getattr(dt, 'tzinfo', None) is not None:
i8 = pydt_to_i8(dt)
i8 = np.int64(dt.timestamp() * 10**9)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm still a little hung up on this. Just to illustrate:

dt1 = datetime.datetime(2020, 1, 17, 16, 1, 9, 136686)
>>> dt2 = pd.Timestamp(np.int64(dt1.timestamp() * 10**9))
>>> dt2.nanosecond
80

I suppose not totally equivalent given the later cast to datetime[D] but this does lose some precision right? If so could lead to some hard to find bugs, but could also misunderstand

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right to be hung up on this, but for a different reason. I had forgotten that the stdlib datetime.timestamp behaves different from pd.Timestamp.timestamp for tz-naive datetimes. I'll give this another look.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i take it back, i was right the first time. this usage is only in the tzaware case, so we're all good

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't do what convet_datetime_to_tsobject does? this doesn't appear to have the same timezone behavior

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't appear to have the same timezone behavior

On the previous line we are checking that we are only doing this in the tzaware case, in which datetime.timestamp and pd.Timestamp.timestamp behave the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as i said i think you should re-use convert_datetime_to_tsobject rather than doing this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this, but it gets appreciably more verbose because we need special handling for datetime vs Timestamp. The version in the PR ATM is correct for both datetime and Timestmap.

dt = tz_convert_single(i8, UTC, dt.tzinfo)
dt = np.int64(dt).astype('datetime64[ns]')
else:
Expand Down
11 changes: 0 additions & 11 deletions pandas/tests/scalar/timestamp/test_timestamp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import pytz
from pytz import timezone, utc

from pandas._libs.tslibs import conversion
from pandas._libs.tslibs.timezones import dateutil_gettz as gettz, get_timezone
import pandas.compat as compat
from pandas.compat.numpy import np_datetime64_compat
Expand Down Expand Up @@ -241,24 +240,20 @@ def test_constructor(self):
for result in [Timestamp(date_str), Timestamp(date)]:
# only with timestring
assert result.value == expected
assert conversion.pydt_to_i8(result) == expected

# re-creation shouldn't affect to internal value
result = Timestamp(result)
assert result.value == expected
assert conversion.pydt_to_i8(result) == expected

# with timezone
for tz, offset in timezones:
for result in [Timestamp(date_str, tz=tz), Timestamp(date, tz=tz)]:
expected_tz = expected - offset * 3600 * 1_000_000_000
assert result.value == expected_tz
assert conversion.pydt_to_i8(result) == expected_tz

# should preserve tz
result = Timestamp(result)
assert result.value == expected_tz
assert conversion.pydt_to_i8(result) == expected_tz

# should convert to UTC
if tz is not None:
Expand All @@ -267,7 +262,6 @@ def test_constructor(self):
result = Timestamp(result, tz="UTC")
expected_utc = expected - offset * 3600 * 1_000_000_000
assert result.value == expected_utc
assert conversion.pydt_to_i8(result) == expected_utc

def test_constructor_with_stringoffset(self):
# GH 7833
Expand Down Expand Up @@ -300,30 +294,25 @@ def test_constructor_with_stringoffset(self):
for result in [Timestamp(date_str)]:
# only with timestring
assert result.value == expected
assert conversion.pydt_to_i8(result) == expected

# re-creation shouldn't affect to internal value
result = Timestamp(result)
assert result.value == expected
assert conversion.pydt_to_i8(result) == expected

# with timezone
for tz, offset in timezones:
result = Timestamp(date_str, tz=tz)
expected_tz = expected
assert result.value == expected_tz
assert conversion.pydt_to_i8(result) == expected_tz

# should preserve tz
result = Timestamp(result)
assert result.value == expected_tz
assert conversion.pydt_to_i8(result) == expected_tz

# should convert to UTC
result = Timestamp(result).tz_convert("UTC")
expected_utc = expected
assert result.value == expected_utc
assert conversion.pydt_to_i8(result) == expected_utc

# This should be 2013-11-01 05:00 in UTC
# converted to Chicago tz
Expand Down