BUG: underflow on Timestamp creation #14433
Conversation
| cdef extern from "headers/stdint.h": | ||
| enum: INT64_MAX | ||
| enum: INT64_MIN | ||
|
|
There was a problem hiding this comment.
shouldn't these be imported from the numpy headers? for consistency
There was a problem hiding this comment.
This is how we do it in lib.pyx, but agreed it probably makes more sense to pull it from the numpy headers.
There was a problem hiding this comment.
ok no big deal
ideally should be consistent across the code
maybe just import into util.pxd and use from there?
| cdef int64_t _NS_LOWER_BOUND = -9223285636854775000LL | ||
| cdef int64_t _NS_UPPER_BOUND = 9223372036854775807LL | ||
| # INT64_MIN is reserved for NaT | ||
| cdef int64_t _NS_LOWER_BOUND = INT64_MIN + 1 |
There was a problem hiding this comment.
why change this?
iirc some platforms don't play nice with this definition
There was a problem hiding this comment.
The old lower bound was higher than it needed to be, because of this issue; although we didn't actually enforce it everywhere. Upper bound is the same.
I can go back to hardcoded numbers, just thought the defined constants were clearer - I assumed INT64_MAX is platform independent (could be wrong)
There was a problem hiding this comment.
it's fine
i don't know why these were hard coded in the first place
There was a problem hiding this comment.
On further look, I'll change the lower bound back - in our compat with python datetime we assume in several places that the min value has a 0 nanosecond unit - I think it could be worked around, but fairly invasive for another 800 ns of span.
Current coverage is 85.26% (diff: 100%)@@ master #14433 diff @@
==========================================
Files 140 140
Lines 50640 50640
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 43175 43176 +1
+ Misses 7465 7464 -1
Partials 0 0
|
|
lgtm. Push again though (you might need to |
|
lgtm. rebase and merge. |
chris-b1
force-pushed
the
timestamp-underflow
branch
from
a180a1a to
4b7674c
Compare
* BUG: underflow on Timestamp creation * undo change to lower bound * change lower bound; but keep rounding to us
* BUG: underflow on Timestamp creation * undo change to lower bound * change lower bound; but keep rounding to us (cherry picked from commit 65362aa)
git diff upstream/master | flake8 --diff