BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865) #8981
Conversation
jreback
added
Bug
Dtype Conversions
|
cc @shoyer was a rabbit-hole! not 100% convinced that the auto-broadcasting strategy will work here, but go for it. |
|
|
||
| # we may be passed reverse ops | ||
| if get_timezone(getattr(self,'tzinfo',None)) != get_timezone(other.tz): | ||
| raise TypeError("Timestamp subtraction must have the same timezones or no timezones") |
There was a problem hiding this comment.
I'm pretty sure timezone comparison is a ValueError... it would be good to be consistent (IMO TypeError is the right choice)
There was a problem hiding this comment.
(Pdb) p dt_tz
datetime.datetime(2013, 1, 1, 0, 0, tzinfo=<DstTzInfo 'US/Eastern' EST-1 day, 19:00:00 STD>)
(Pdb) p dt
datetime.datetime(2013, 1, 1, 0, 0)
(Pdb) p dt_tz - dt
*** TypeError: TypeError("can't subtract offset-naive and offset-aware datetimes",)
datetime-datetime has this as a TypeError (that's why I changed it, originally I had it as a ValueError as well)
There was a problem hiding this comment.
OK, so maybe you should also change this for comparisons on line 813 above?
There was a problem hiding this comment.
I guess its bad when I change from ValueError to TypeError and no tests break :)
It was almost all TypeError, so this is the right move.
There was a problem hiding this comment.
actually was a failed test so that is good :)
|
generally looks pretty good to me! |
|
@shoyer great! still would love to have you refactor when you have to time to use more array broadcasting stuff. |
…telike-datelike ops with timezones (GH8865)
BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865)
closes #8865