BUG: Addition raises TypeError if Period is on rhs #13069
Conversation
sinhrks
added
Bug
Numeric Operations
|
didn't you make this an issue somewhere? (or maybe just a comment) |
| if isinstance(other, (ABCDatetimeIndex, ABCSeries)): | ||
| return other + self | ||
| elif isinstance(other, Period): |
|
No existing issue. Updated the top comment. |
| """ | ||
|
|
||
| def __radd__(self, other): | ||
| # __radd__ on cython extension types like _Period is not used, so |
There was a problem hiding this comment.
is there a reference somewhere on this? just curious
There was a problem hiding this comment.
I couldn't find as long as I searched. We can fix Timestamp if there is a workaround.
There was a problem hiding this comment.
on what I mean is where you saw that it didn't work?
There was a problem hiding this comment.
Once I tried to add __radd__ to cdef Period and found it doesn't work. Then noticed _Timestamp impl. I'm not sure where _Timestamp impl derived from.
There was a problem hiding this comment.
There was a problem hiding this comment.
ahh so this means you can leave Period and we don't need _Period. That's only necessary for Timestamp for example because we are extending a c class itself
here the class is already in cython
but what it does mean is that add needs to handle reversed args directly (and similarly for other arith ops)
There was a problem hiding this comment.
I read the doc something like below. This looks work removing _Period class.
def __add__(self, other):
if isinstance(self, Period):
...
elif isinstance(other, Period):
return other + self
else:
...
0d31d32 to
cdcd3b3
Compare
Current coverage is 84.14%@@ master #13069 diff @@
==========================================
Files 137 137
Lines 50261 50231 -30
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 42288 42265 -23
+ Misses 7973 7966 -7
Partials 0 0
|
BUG: Addition raises TypeError if Period is on rhs
|
thanks @sinhrks |
git diff upstream/master | flake8 --diffPeriodaddition raisesTypeErrorifPeriodis on right hand side.Expected