Skip to content

BUG: Fix Timestamp type checks to work with subclassed datetime (#25851) #25853

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 31 commits into from
Apr 5, 2019
Merged

BUG: Fix Timestamp type checks to work with subclassed datetime (#25851) #25853

Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
2e5a160
BUG: Fix Timestamp type checks to work with subclassed datetime (#25851)
ArtificialQualia Mar 23, 2019
82b2420
increase spacing in comment
ArtificialQualia Mar 23, 2019
4447746
fix spacing
ArtificialQualia Mar 23, 2019
2d20375
fix import formatting
ArtificialQualia Mar 24, 2019
8e8d816
move _Timestamp to separate file to prevent circular import
ArtificialQualia Mar 28, 2019
4fd35ac
fix indenting
ArtificialQualia Mar 28, 2019
354d3a3
fix line spacing
ArtificialQualia Mar 28, 2019
600be4f
kickoff new build
ArtificialQualia Mar 28, 2019
7a60942
updates for PR review
ArtificialQualia Mar 29, 2019
dd5d492
Merge branch 'master' into update_timestamp_checks
ArtificialQualia Mar 29, 2019
46f306d
fix isort error
ArtificialQualia Mar 29, 2019
7268a91
Merge branch 'update_timestamp_checks' of https://github.com/Artifici…
ArtificialQualia Mar 29, 2019
ff0ddef
fix isort error
ArtificialQualia Mar 29, 2019
c54ee53
force rebuild
ArtificialQualia Mar 30, 2019
6dc805b
PR updates
ArtificialQualia Mar 30, 2019
789e121
Merge branch 'master' into update_timestamp_checks
ArtificialQualia Mar 30, 2019
6a66238
parameterize test
ArtificialQualia Mar 30, 2019
12a53be
Merge branch 'update_timestamp_checks' of https://github.com/Artifici…
ArtificialQualia Mar 30, 2019
7da7684
PR updates
ArtificialQualia Mar 31, 2019
dc0f806
fix namespace test for new api
ArtificialQualia Mar 31, 2019
0cad612
fix typo
ArtificialQualia Mar 31, 2019
a1cb913
Merge branch 'master' into update_timestamp_checks
ArtificialQualia Apr 5, 2019
ee9d2d5
merge changes into _timestamp
ArtificialQualia Apr 5, 2019
560ef6f
change _timestamp files to c_timestamp
ArtificialQualia Apr 5, 2019
92a9c52
Merge branch 'master' into update_timestamp_checks
ArtificialQualia Apr 5, 2019
b87aa99
use new tzconversion.tz_convert_single merged function
ArtificialQualia Apr 5, 2019
11a091a
move imports to fix import exceptions
ArtificialQualia Apr 5, 2019
4896568
change imports
ArtificialQualia Apr 5, 2019
43ec600
fix api test
ArtificialQualia Apr 5, 2019
9795807
force rebuild
ArtificialQualia Apr 5, 2019
ded5e69
force rebuild
ArtificialQualia Apr 5, 2019
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.25.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ Sparse
Other
^^^^^

- Improved :class:`Timestamp` type checking in various datetime functions to prevent exceptions when using a subclassed `datetime` (:issue:`25851`)
- Bug in :class:`Series` and :class:`DataFrame` repr where ``np.datetime64('NaT')`` and ``np.timedelta64('NaT')`` with ``dtype=object`` would be represented as ``NaN`` (:issue:`25445`)
-
-
Expand Down
5 changes: 2 additions & 3 deletions pandas/_libs/tslib.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import cython

from cpython.datetime cimport (PyDateTime_Check, PyDate_Check,
PyDateTime_CheckExact,
PyDateTime_IMPORT,
timedelta, datetime, date, time)
# import datetime C API
Expand All @@ -19,6 +18,7 @@ import pytz
from pandas._libs.util cimport (
is_integer_object, is_float_object, is_datetime64_object)

from pandas._libs.tslibs.c_timestamp cimport _Timestamp

from pandas._libs.tslibs.np_datetime cimport (
check_dts_bounds, npy_datetimestruct, _string_to_dts, dt64_to_dtstruct,
Expand Down Expand Up @@ -539,8 +539,7 @@ cpdef array_to_datetime(ndarray[object] values, str errors='raise',
'datetime64 unless utc=True')
else:
iresult[i] = pydatetime_to_dt64(val, &dts)
if not PyDateTime_CheckExact(val):
# i.e. a Timestamp object
if isinstance(val, _Timestamp):
iresult[i] += val.nanosecond
check_dts_bounds(&dts)

Expand Down
1 change: 1 addition & 0 deletions pandas/_libs/tslibs/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
from .timestamps import Timestamp
from .timedeltas import delta_to_nanoseconds, ints_to_pytimedelta, Timedelta
from .tzconversion import tz_convert_single
from .c_timestamp import maybe_integer_op_deprecated # isort:skip
Copy link
Contributor

@jreback jreback Apr 5, 2019

Choose a reason for hiding this comment

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

do we really need this function here? this should not be exposed to the rest of the codebase (or directly imported if it actually is needed)
Copy link
Contributor

@jreback jreback Apr 5, 2019

Choose a reason for hiding this comment

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

maybe just expose it in timestamp.pyx itself to make it easier to import
Copy link
Contributor Author

@ArtificialQualia ArtificialQualia Apr 5, 2019

Choose a reason for hiding this comment

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

It was exposed previously, and is only used in pandas/core/arrays/datetimelike.py. _Timestamp itself makes use of it, so it has to be in c_timestamp.pyx otherwise we would have a circular import.

I can change the import in datetimelike.py to directly import it if you want.
Copy link
Contributor

@jreback jreback Apr 5, 2019

Choose a reason for hiding this comment

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

sorry made the comment in the wrong place. yes just import it directly in dateimelike.py and don't put it in tslibs/__init__.py
19 changes: 19 additions & 0 deletions pandas/_libs/tslibs/c_timestamp.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# -*- coding: utf-8 -*-

from cpython.datetime cimport datetime

from numpy cimport int64_t

cdef class _Timestamp(datetime):
cdef readonly:
int64_t value, nanosecond
object freq
list _date_attributes
cpdef bint _get_start_end_field(self, str field)
Copy link
Contributor

@jreback jreback Apr 5, 2019

Choose a reason for hiding this comment

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

future PR we should rename the 'private' functions to public if they are actually exposed (the _ leading ones)
cpdef _get_date_name_field(self, object field, object locale)
cdef int64_t _maybe_convert_value_to_local(self)
cpdef to_datetime64(self)
cdef _assert_tzawareness_compat(_Timestamp self, datetime other)
cpdef datetime to_pydatetime(_Timestamp self, bint warn=*)
cdef bint _compare_outside_nanorange(_Timestamp self, datetime other,
int op) except -1
Loading