BUG: catch overflow in timestamp + offset operations #15126
Conversation
pandas/tslib.pyx Outdated
| | ||
| elif isinstance(other, timedelta) or hasattr(other, 'delta'): | ||
| nanos = _delta_to_nanoseconds(other) | ||
| try: |
There was a problem hiding this comment.
cc @shoyer
since IIRC you have some expertise doing this.
I could not get this to raise the exception at all (from cython), rather it always wanted to simply call __radd__. The only way is to return NotImplemented.
any better way to do this?
There was a problem hiding this comment.
Cython really doesn't let you raise any errors from __add__ without calling __radd__ on the other object? That is surprising -- and disappointing!
It is true that Cython will use __add__ for both Python's __add__ and __radd__, so self in this method may not be a Timestamp instance:
http://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#arithmetic-methods
There was a problem hiding this comment.
ok I sorted. Its very odd that it doesn't raise the actual exception.
jreback force-pushed the inc branch 2 times, most recently from 4146495 to 04454ae Compare 
Current coverage is 85.53% (diff: 100%)@@ master #15126 diff @@ ========================================== Files 145 145 Lines 51274 51281 +7 Methods 0 0 Messages 0 0 Branches 0 0 ========================================== + Hits 43856 43865 +9 + Misses 7418 7416 -2 Partials 0 0
|
| # Timestamp can handle tz and nano sec, thus no need to use apply_wraps | ||
| if isinstance(other, (datetime, np.datetime64, date)): | ||
| if isinstance(other, Timestamp): | ||
| result = other.__add__(self) |
There was a problem hiding this comment.
Do you really need to call __add__ here rather than using +?
Generally calling double-underscore methods directly is an anti-pattern, so if so this definitely deserves a comment to explain.
There was a problem hiding this comment.
added a comment. The + causes it to segfault. I think its because of a fast-path in the interpreter, or maybe a cython bug. If there is an exception during the the operation. Then we compound this by trying the reversed ops in .apply and the cycle repeats. But I think its too hard (maybe impossible) to change this.
xref statsmodels/statsmodels#3374 Author: Jeff Reback <jeff@reback.net> Closes pandas-dev#15126 from jreback/inc and squashes the following commits: 923eaa2 [Jeff Reback] BUG: catch overflow in timestamp + offset operations
3 participants
xref statsmodels/statsmodels#3374