Repository: pandas-dev/pandas
Pull request: BUG: Fix error in replace with strings that are large numbers (#25616)
- [x] closes #25616
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

See discussion in #25616.

When `.replace` saw a value that looks like an int, it would try to convert it even if it caused an `OverflowError`.  This issue is only happening in newer versions of pandas due to the addition of `coerce_to_target_dtype` in `_replace_coerce`.  `coerce_to_target_dtype` is required to fix a lot of other issues, so the fix here was to prevent a coercion to an int that would cause an OverflowError by catching that exception, allowing the values to remain as objects.

I tried to play around with `coerce_to_target_dtype` as well (moving it until after the replace, only doing it when covert is True, etc.) but this caused various other coercion and replace tests to fail, so I left that untouched.

Tests have been added for both cases where I found `OverflowError` could occur with `replace`.

Problem statement:
pandas.DataFrame.replace seems taking number string as integer and run into overflow error
#### Code Sample

```python
import pandas as pd

x = {'user_id':['100000715097692381911', 
                '100003840837471130074'], 
     'item_id': [1, 2]
     }
dfx = pd.DataFrame(x)
dfx['user_id'].replace(
    {
     '100000715097692381911': 0, 
     '100003840837471130074': 1
     }, inplace=True)
```
#### Problem description
I got overflow error:

File "/home/grain/ml/lib/python3.6/site-packages/pandas/core/dtypes/cast.py", line 683, in astype_nansafe
    return lib.astype_intsafe(arr.ravel(), dtype).reshape(arr.shape)
  File "pandas/_libs/lib.pyx", line 546, in pandas._libs.lib.astype_intsafe
OverflowError: Python int too large to convert to C long

*This error occurs on my laptop with pandas 0.24.1, python 3.6.7. The example code can handle the replacement correctly on another computer with an older version pandas 0.20.3.*

#### Expected Output

Since I loaded big id numbers like '100000715097692381911' as string type, the pandas.DataFrame.replace() method should replace it with the corresponding value in the dictionary. But I got overflow error. It seemed pandas took the id string as integer?

And if I modify just the first element of 'user_id' column to 's100000715097692381911', then the code won't run into overflow.

#### Output of ``pd.show_versions()``

<details>

[paste the output of ``pd.show_versions()`` here below this line]
INSTALLED VERSIONS
------------------
commit: None
python: 3.6.7.final.0
python-bits: 64
OS: Linux
OS-release: 4.15.0-46-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.24.1
pytest: None
pip: 19.0.3
setuptools: 40.6.3
Cython: None
numpy: 1.16.0
scipy: 1.2.0
pyarrow: None
xarray: None
IPython: 7.2.0
sphinx: None
patsy: None
dateutil: 2.7.5
pytz: 2018.3
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 3.0.2
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml.etree: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
gcsfs: None

</details>

Hints from issue discussion:
Hmm ok that is strange - investigation and PRs would certainly be welcome
I'm new to working on pandas, so please note that some of the below may be incorrect.

I've looked into this a bit, and it looks like the cause is code added in #21477 in `Block._replace_coerce`, specifically the `coerce_to_target_dtype` function.

This didn't exist in the older version of pandas that the above code works on, and conversion was handled totally by the `ObjectBlock.convert` function.  At some point, it seems that function no longer met all the needs of converting, and `coerce_to_target_dtype` was introduced into the replace process to meet some of those needs.  However, that function tries too forcefully to convert the original Block (and it does so before it even finishes replacing values!), and thus we get an OverflowError in this case.

An easy fix to this is to just add `OverflowError ` to the list of errors `coerce_to_target_dtype` can catch ([here](https://github.com/pandas-dev/pandas/blob/master/pandas/core/internals/blocks.py#L1082)), which would fix this and retain other existing functionality.  However, I'm not sure if `coerce_to_target_dtype` is *meant* to fail in the case of an int that is too large.

If we can't change that, a much more narrow fix would be to only call `coerce_to_target_dtype` after the last replacement is done, but that doesn't some other cases that the old code could handle (for instance: replacing one of those values in the example instead of both of them).

I couldn't see a fix we could make to `ObjectBlock.convert` to handle all conversions without needing to call `coerce_to_target_dtype`.  I'm guessing this has to do with numpy changes, or perhaps changes to `maybe_convert_objects`, but I'm not sure.
@ArtificialQualia thanks a lot for your input!

> However, that function tries too forcefully to convert the original Block (and it does so before it even finishes replacing values!

I haven't stepped through the code myself but this seems to be the actual root issue - have you tried debugging this behavior to see if a fix can be applied to prevent this?
@WillAyd 

I did try both the fixes I mentioned in my comment.  They fix the issue mentioned in this issue, as well as passing all the tests in `tests\series\test_replace.py`.

If you want my recommendation, I would add `OverflowError` to `coerce_to_target_dtype` @ [line 1082](https://github.com/pandas-dev/pandas/blob/master/pandas/core/internals/blocks.py#L1082) and also move the call to `coerce_to_target_dtype` in `_replace_coerce` to after `putmask` so the coercion happens after the replacement.  Possibly also add in a `if convert:` around the `coerce_to_target_dtype` call so it only happens on the last entry, which is similar to how `ObjectBlock.convert` works which I'm assuming is for performance reasons.
@ArtificialQualia if you'd like to submit a PR would probably be easier to review and give you feedback with that. Thanks again!
No problem, I'll submit a PR.

Commit message: BUG: Fix error in replace with strings that are large numbers (#25616)

Patch to review:
diff --git a/doc/source/whatsnew/v0.24.2.rst b/doc/source/whatsnew/v0.24.2.rst
index 2c6d1e01ed89b..e269a98e3373f 100644
--- a/doc/source/whatsnew/v0.24.2.rst
+++ b/doc/source/whatsnew/v0.24.2.rst
@@ -32,6 +32,7 @@ Fixed Regressions
 - Fixed regression in creating a period-dtype array from a read-only NumPy array of period objects. (:issue:`25403`)
 - Fixed regression in :class:`Categorical`, where constructing it from a categorical ``Series`` and an explicit ``categories=`` that differed from that in the ``Series`` created an invalid object which could trigger segfaults. (:issue:`25318`)
 - Fixed pip installing from source into an environment without NumPy (:issue:`25193`)
+- Fixed regression in :func:`replace` where large strings of numbers would be coerced into int, causing an ``OverflowError`` (:issue:`25616`)
 
 .. _whatsnew_0242.enhancements:
 
diff --git a/pandas/core/internals/blocks.py b/pandas/core/internals/blocks.py
index ada663556899b..0375f782badcc 100644
--- a/pandas/core/internals/blocks.py
+++ b/pandas/core/internals/blocks.py
@@ -1079,7 +1079,7 @@ def coerce_to_target_dtype(self, other):
 
         try:
             return self.astype(dtype)
-        except (ValueError, TypeError):
+        except (ValueError, TypeError, OverflowError):
             pass
 
         return self.astype(object)
@@ -3210,7 +3210,7 @@ def _putmask_smart(v, m, n):
                 nv = v.copy()
                 nv[m] = nn_at
                 return nv
-    except (ValueError, IndexError, TypeError):
+    except (ValueError, IndexError, TypeError, OverflowError):
         pass
 
     n = np.asarray(n)
diff --git a/pandas/tests/series/test_replace.py b/pandas/tests/series/test_replace.py
index 40b28047080da..6a4e3a25b486e 100644
--- a/pandas/tests/series/test_replace.py
+++ b/pandas/tests/series/test_replace.py
@@ -181,6 +181,20 @@ def check_replace(to_rep, val, expected):
         tr, v = [3, 4], [3.5, True]
         check_replace(tr, v, e)
 
+        # GH 25616
+        # casts to object without Exception due to OverflowError
+        e = pd.Series([0, 1, 2, '100000000000000000000', 4])
+        tr, v = [3], ['100000000000000000000']
+        check_replace(tr, v, e)
+
+        # GH 25616
+        # casts to object without Exception due to OverflowError
+        original = pd.Series([0, '100000000000000000000',
+                                 '100000000000000000001'])
+        result = original.replace(['100000000000000000000'], [1])
+        expected = pd.Series([0, 1, '100000000000000000001'])
+        tm.assert_series_equal(result, expected)
+
         # test an object with dates + floats + integers + strings
         dr = pd.date_range('1/1/2001', '1/10/2001',
                            freq='D').to_series().reset_index(drop=True)


Code review comments:
diff_hunk: @@ -32,6 +32,7 @@ Fixed Regressions
 - Fixed regression in creating a period-dtype array from a read-only NumPy array of period objects. (:issue:`25403`)
 - Fixed regression in :class:`Categorical`, where constructing it from a categorical ``Series`` and an explicit ``categories=`` that differed from that in the ``Series`` created an invalid object which could trigger segfaults. (:issue:`25318`)
 - Fixed pip installing from source into an environment without NumPy (:issue:`25193`)
+- Fixed regression in :func:`replace` where large strings of numbers would be coerced into int, causing an ``OverflowError`` (:issue:`25616`)
path: doc/source/whatsnew/v0.24.2.rst
text: @user1:
use DataFrame.replace as what you have won't render. use double-backticks around int.