BUG: Prevent addition overflow with TimedeltaIndex
State: closed

Expands checked-add array addition introduced in #14237 to include all other addition cases (i.e.
`TimedeltaIndex` and `Timedelta`).  Follow-up to #14453.

Labels:
name: Bug
name: Timedelta
description: Timedelta data type

Comments:
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14816?src=pr) is 85.33% (diff: 89.18%)
> Merging [#14816](https://codecov.io/gh/pandas-dev/pandas/pull/14816?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **<.01%**

```diff
@@             master     #14816   diff @@
==========================================
  Files           144        144          
  Lines         51043      51058    +15   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43552      43569    +17   
+ Misses         7491       7489     -2   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [dd8cba2...a086db6](https://codecov.io/gh/pandas-dev/pandas/compare/dd8cba27677895e80768f22f6992e58046dcc3bf...a086db674f0accb6c6b24729ee482d7fbaf08ec4?src=pr)
Can you show the results of the related benchmarks before/after PR?
Setup script:
~~~python
import pandas as pd
import numpy as np

checked_add = pd.core.nanops._checked_add_with_arr

arr = np.arange(1000000)
arrpos = np.arange(1000000)
arrneg = np.arange(-1000000, 0)
arrmixed = np.array([1, -1]).repeat(500000)
~~~
Timing info is with 100 loops, best of 3

On `master`:
* `checked_add(arr, 1)`: 8.39 ms / loop
* `checked_add(arr, -1)`: 8.38 ms / loop
* `checked_add(arr, 0)`: 8.37 ms / loop
* `checked_add(arr, arrpos)`: 9.53 ms / loop
* `checked_add(arr, arrneg)`: 9.34 ms / loop
* `checked_add(arr, arrmixed)`: 14.9 ms / loop

On `PR`:
* `checked_add(arr, 1)`: 8.35 ms / loop
* `checked_add(arr, -1)`: 8.34 ms / loop
* `checked_add(arr, 0)`: 8.42 ms / loop
* `checked_add(arr, arrpos)`: 10.1 ms / loop
* `checked_add(arr, arrneg)`: 9.6 ms / loop
* `checked_add(arr, arrmixed)`: 16.8 ms / loop
@jreback , @jorisvandenbossche : How does the performance look to you both?  If it looks good, should be ready to merge if there are no other concerns.
Those timings look good. Can you add a test for timedelta where the overflow is raised? (for the cases where you added the check)
you didn't address my api comments
this is inconsistent with the rest of the code
@jreback : I did address your API comments <a href="https://github.com/pandas-dev/pandas/pull/14816#discussion_r91802356">here</a>.  It also explains why I don't think consistency is an issue here.  I don't understand why you want to make this internal function "consistent," especially when it doesn't really make sense to align it with the rest of the other functions.
@jreback @jorisvandenbossche : Do you have anything more to add about this API consistency issue, especially in light of my response?
As a possible way out of the discussion: @jreback, as your main point was a lack of consistency with the rest of the functions in `nanops.py`, would the current version be more OK to you when we moved it to another location (and actually, this function is not really a "nanop" anyway, it is a adapted `add` to deal with integer overflow in the first place, not NaNs). Eg `core/algorithms.py` or somewhere else.
would be ok with a move to core/algorithms.py and api as currently (meaning the last iteration)
Changed `_checked_add_with_arr` to `checked_add_with_arr` in `core/algorithms.py` <strike>since we are exposing it in `core/api.py`.  Added `whatsnew` to `v0.20.0` since this is technically an API change.</strike>
@gfyoung moving to an internal pandas module and exposing as a public method are 2 very separate and distinct things.
@jreback : Yes, I am aware, so then what did you mean by "a move to `core/algorithms` and `api`" then?
api as you have currently, e.g. ``(a, b, a_mask, b_mask)`` (though I think you have something slightly different)
@jreback : Ah, I see.  Signature is `(arr, b, arr_mask=None, b_mask=None)`.
@jreback , @jorisvandenbossche : Moved checked add function to `core/algorithms.py`, and everything is still passing.  Ready to merge if there are no other concerns.
@jorisvandenbossche :

~~~
· Discovering benchmarks
· Running 16 total benchmarks (1 commits * 1 environments * 16 benchmarks)
[  0.00%] ·· Building for existing-py_home_user_miniconda3_envs_pandas-dev_bin_python
[  0.00%] ·· Benchmarking existing-py_home_user_miniconda3_envs_pandas-dev_bin_python
[  6.25%] ··· Running ...ms.Algorithms.time_add_overflow_both_arg_nan    16.24ms
[ 12.50%] ··· Running ...s.Algorithms.time_add_overflow_first_arg_nan    15.98ms
[ 18.75%] ··· Running ...ithms.Algorithms.time_add_overflow_mixed_arr    16.67ms
[ 25.00%] ··· Running algorithms.Algorithms.time_add_overflow_neg_arr     8.93ms
[ 31.25%] ··· Running ...thms.Algorithms.time_add_overflow_neg_scalar     8.02ms
[ 37.50%] ··· Running algorithms.Algorithms.time_add_overflow_pos_arr     9.03ms
[ 43.75%] ··· Running ...thms.Algorithms.time_add_overflow_pos_scalar     8.07ms
[ 50.00%] ··· Running ....Algorithms.time_add_overflow_second_arg_nan    15.96ms
[ 56.25%] ··· Running ...hms.Algorithms.time_add_overflow_zero_scalar     8.12ms
[ 62.50%] ··· Running algorithms.Algorithms.time_duplicated_float        18.64ms
[ 68.75%] ··· Running algorithms.Algorithms.time_duplicated_int          15.32ms
[ 75.00%] ··· Running ...rithms.Algorithms.time_duplicated_int_unique    50.73μs
[ 81.25%] ··· Running algorithms.Algorithms.time_factorize_float          7.59ms
[ 87.50%] ··· Running algorithms.Algorithms.time_factorize_int            7.49ms
[ 93.75%] ··· Running algorithms.Algorithms.time_factorize_string        29.15ms
[100.00%] ··· Running algorithms.Algorithms.time_match_strings            1.91ms
~~~
@jreback , @jorisvandenbossche : Addressed all of the comments and ran the benchmarks as requested.  Everything is green, so ready to merge if there is nothing else.
@gfyoung thanks!