Spending some time on #12534 the only fixes I'm seeing are pretty invasive. We need to use checked_add_with_arr
to correctly catch overflows, and for that we need isna(...)
masks. But the wrapping/redirection that goes on within ops._arith_method_SERIES
make that problematic. In particular the scope in which the masks are defined (for datetime/timedelta dtypes) is different from the scope in which we need to do the overflow checking. Passing those masks back and forth across scopes gets ugly in a hurry.
I'd rather just define __add__
, __sub__
, __radd__
, __rsub__
methods specific to datetimelike-dtypes and have Series dispatch to those when appropriate. Any thoughts?
Comment From: gfyoung
@jbrockmendel : That's an interesting suggestion. Since this is pretty invasive, I'm not sure if we would actually tackle this for some time. Would be good for 1.0
though.
Comment From: gfyoung
@jbrockmendel : Oh, don't close this! :smile: Keep it open. This is a very valid issue to raise. It just may not be addressed for some time give the invasiveness.
Comment From: jbrockmendel
The idea was to roll it into #18824. I’m happy either way.
Comment From: gfyoung
Ah, gotcha. Just for future reference, add a comment that you're doing this so that we're all on the same page. :smile: