BUG: maximum recursion error when replacing empty lists #22083

minggli · 2018-07-27T14:01:38Z

closes Pandas Series: maximum recursion error when replacing empty lists #21977
closes Series/DataFrame.replace crashes without exception in some cases (concerning lists) #19266
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Internal mechanism in replace method is intertwined and can cause RecursionError (not very helpful). Discussed in #21977, there doesn't appear to be first class support for ExtensionArray yet and this issue is right now treated as error handling, having investigated treating it as new use case but not easily co-exist without large change.

The RecursionError is caused by Block.replace and ObjectBlock.replace calling each other under certain circumstances such as in the linked issues.

New approach is now to prevent re-casting of blocks to object blocks if it is already an ObjectBlock and instead raise the error caught as it is, because if it's an ObjectBlock already then ValueError and TypeError raised won't go away when re-casted to object and re-try, resulting in infinite loop and RecursionError.

jreback

can you add the example as a test

jreback · 2018-07-28T12:46:55Z

pandas/core/internals/blocks.py

@@ -1000,8 +999,8 @@ def putmask(self, mask, new, align=True, inplace=False, axis=0,
                if not (mask.shape[-1] == len(new) or
                        mask[mask].shape[-1] == len(new) or
                        len(new) == 1):
-                    raise ValueError("cannot assign mismatch "


does this need to change? (or is this what is actually causing the issue); this is not exposed to the user, correct?

Yes this ValueError caused Block.replace and ObjectBlock.replace to form infinite loop and ultimately raise RecursionError. It's because Block.replace catches exception include ValueError and if it fails, delegate replace to ObjectBlock. But ObjectBlock.replace also uses super(ObjectBlock, self), hence the loop. The exception handling includes TypeError and ValueError and they are both depended on (as appear to me) by other casting cases.

Open to suggestion, not sure a better way to handle, at the moment this change is needed to address the issue in question.

This can be exposed to user when:

import pandas as pd s = pd.Series(range(10)) mask = s > 5 s[mask] = [5, 4, 3, 2, 1]

do you know why we are catching ValueError at a higher level, maybe can get away with only catching TypeError and let ValueError buble up?

Good point and it appears below test (only this test) fails when we only catch TypeError:

_________________ TestDataFrameReplace.test_replace_datetimetz _________________
# coerce to object
result = df.copy()
result.iloc[1, 0] = np.nan
result = result.replace(
{'A': pd.NaT}, Timestamp('20130104', tz='US/Pacific'))

pandas/tests/frame/test_replace.py:1060:

elif isinstance(other, (np.datetime64, datetime, date)): other = tslibs.Timestamp(other) tz = getattr(other, 'tz', None) # test we can have an equal time zone if tz is None or str(tz) != str(self.values.tz): raise ValueError("incompatible or non tz-aware value")

E ValueError: incompatible or non tz-aware value

pandas/core/internals/blocks.py:2898: ValueError

It fails because of above ValueError which is passed on to ObjectBlock.replace as part of error handling in Block.replace. so it appears it is expected to be handled after the block is casted as object and try again.

@jreback made some changes now we don't touch above error but instead make sure we don't enter infinite loop.

codecov · 2018-07-28T22:52:05Z

Codecov Report

Merging #22083 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #22083      +/-   ##
==========================================
+ Coverage   92.06%   92.06%   +<.01%     
==========================================
  Files         169      169              
  Lines       50694    50698       +4     
==========================================
+ Hits        46671    46675       +4     
  Misses       4023     4023

Flag	Coverage Δ
#multiple	`90.47% <100%> (ø)`	⬆️
#single	`42.32% <25%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/missing.py	`91.7% <100%> (+0.04%)`	⬆️
pandas/core/internals/blocks.py	`94.45% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2156431...2bbd097. Read the comment docs.

minggli · 2018-07-28T23:06:17Z

added test cases too from two issues linked. 🚀

@jreback gentle nudge.

jreback · 2018-08-01T22:26:56Z

pandas/core/internals/blocks.py

+                    # GH 19266 and GH 21977
+                    # ValueError triggers try except block in Block.replace
+                    # causing RecursionError
+                    raise Exception("cannot assign mismatch length "


can you make this a bit more informative from a user POV?

sure. this change here is exposed to user so when one tries to assign an array of n-dimension to mask of m-dimension, when m != n, this error throws.

>>> import pandas as pd >>> s = pd.Series(range(10)) >>> mask = s > 5 >>> s[mask].shape (4,) >>> s[mask] = [5, 4, 3, 2, 1] File "/Users/mingli/.virtualenvs/cust-value/lib/python3.5/site-packages/pandas/core/internals.py", line 1006, in putmask raise ValueError("cannot assign mismatch " ValueError: cannot assign mismatch length to masked array

This is also used internally by replace and ValueError it raises is causing infinite loop as per linked issues. Whilst changing that to not ValueError will solve it, it also makes it less clear and potentially breaks users' codes if they expect to catch ValueError running above code snippet.

Do you think it's ok to use Exception here as it would solve the issues linked but users might have to change their code?

this is reverted.

pep8speaks · 2018-08-06T10:59:49Z

Hello @minggli! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 07, 2018 at 13:23 Hours UTC

…cursion * upstream/master: Added 0.23.4 whatsnew [ci skip] (pandas-dev#22214)

minggli · 2018-08-06T14:08:24Z

@jreback @WillAyd I think PR is done, comments welcome.

WillAyd · 2018-08-06T16:22:48Z

pandas/tests/frame/test_replace.py

@@ -603,6 +603,20 @@ def test_replace_list(self):

        assert_frame_equal(res, expec)

+    def test_replace_with_empty_list(self):
+        # GH 21977
+        s = pd.Series([['a', 'b'], np.nan, [1]])


Did you mean to include an empty list in this test case (exists in the series module one)

good point!

WillAyd · 2018-08-06T16:23:27Z

doc/source/whatsnew/v0.24.0.txt

@@ -667,7 +667,8 @@ Reshaping
 - Bug in :meth:`DataFrame.replace` raises RecursionError when converting OutOfBounds ``datetime64[ns, tz]`` (:issue:`20380`)
 - :func:`pandas.core.groupby.GroupBy.rank` now raises a ``ValueError`` when an invalid value is passed for argument ``na_option`` (:issue:`22124`)
 - Bug in :func:`get_dummies` with Unicode attributes in Python 2 (:issue:`22084`)
-
+- Bug in :meth:`DataFrame.replace` raises RecursionError when replace empty lists (:issue:`22083`)


Very minor nits but put RecursionError in double backticks and change replace to replacing

thanks! changed.

WillAyd

lgtm

jreback

minor comments, ping on green.

jreback · 2018-08-07T12:53:58Z

pandas/core/internals/blocks.py

-                                 convert=convert)
+            # GH 22083, TypeError or ValueError occurred within error handling
+            # causes infinite loop. Cast and retry only if not objectblock.
+            if self.dtype == 'object':


can you use is_object_dtype

jreback · 2018-08-07T12:54:27Z

pandas/core/internals/blocks.py

+            # causes infinite loop. Cast and retry only if not objectblock.
+            if self.dtype == 'object':
+                raise
+            else:


don't need this else here

jreback · 2018-08-07T15:55:48Z

thanks @minggli

…2083)

minggli added 2 commits July 27, 2018 16:24

minor docstring

a3f688a

prevent infinite recursion error

c97b2ee

minggli force-pushed the bugfix/replace_recursion branch from 621485b to c97b2ee Compare July 27, 2018 15:45

minggli added 2 commits July 27, 2018 17:16

negative mask if no match or na

941fccb

revert generic

389ebcd

jreback requested changes Jul 28, 2018

View reviewed changes

jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Jul 28, 2018

minggli added 5 commits July 28, 2018 15:25

revert missing

5573658

add test case for series.replace

e7dad7e

boolean mask guaranteed if None

309216c

add test case for frame

402d983

update series test

3604031

minggli force-pushed the bugfix/replace_recursion branch from 60de53c to 4747c98 Compare July 28, 2018 17:51

use different error to avoid recursion

001c3a0

minggli force-pushed the bugfix/replace_recursion branch from 9506cc4 to 001c3a0 Compare July 28, 2018 17:54

minggli added 2 commits July 28, 2018 20:41

modify tests to expect intended error

91100f7

change error type and avoid infinite loop

8271576

jreback added this to the 0.24.0 milestone Aug 1, 2018

jreback requested changes Aug 1, 2018

View reviewed changes

minggli closed this Aug 2, 2018

minggli reopened this Aug 2, 2018

minggli closed this Aug 5, 2018

minggli reopened this Aug 5, 2018

minggli force-pushed the bugfix/replace_recursion branch from 53bfe6d to 9e27a38 Compare August 5, 2018 18:04

revert test_boolean

167a3b7

minggli force-pushed the bugfix/replace_recursion branch from fa1a1e5 to 167a3b7 Compare August 6, 2018 10:54

revert type of error

f657ae3

only retry if the block is not ObjectBlock

c8f4f72

minggli force-pushed the bugfix/replace_recursion branch from 3db333f to c8f4f72 Compare August 6, 2018 11:04

minggli and others added 4 commits August 6, 2018 12:09

add comment

d3c3236

Merge branch 'master' into bugfix/replace_recursion

899724f

Merge remote-tracking branch 'upstream/master' into bugfix/replace_re…

0c2bae3

…cursion * upstream/master: Added 0.23.4 whatsnew [ci skip] (pandas-dev#22214)

add whatsnew

a8020b9

WillAyd requested changes Aug 6, 2018

View reviewed changes

minggli added 2 commits August 6, 2018 17:37

minor change in whatsnew

201adf6

add empty list

c4a4a8b

WillAyd approved these changes Aug 6, 2018

View reviewed changes

jreback requested changes Aug 7, 2018

View reviewed changes

minor refactor

2bbd097

jreback approved these changes Aug 7, 2018

View reviewed changes

jreback merged commit 96b4873 into pandas-dev:master Aug 7, 2018

minggli deleted the bugfix/replace_recursion branch August 7, 2018 16:00

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

BUG: maximum recursion error when replacing empty lists (pandas-dev#2…

9454f36

…2083)

mroeschke mentioned this pull request May 29, 2019

RecursionError with DataFrame.replace() where to_replace is an empty list #26567

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: maximum recursion error when replacing empty lists #22083

BUG: maximum recursion error when replacing empty lists #22083

minggli commented Jul 27, 2018 •

edited

Loading

jreback left a comment

jreback Jul 28, 2018

minggli Jul 28, 2018 •

edited

Loading

jreback Aug 2, 2018

minggli Aug 2, 2018 •

edited

Loading

minggli Aug 6, 2018

codecov bot commented Jul 28, 2018 •

edited

Loading

minggli commented Jul 28, 2018 •

edited

Loading

jreback Aug 1, 2018

minggli Aug 2, 2018 •

edited

Loading

minggli Aug 6, 2018

pep8speaks commented Aug 6, 2018 •

edited

Loading

minggli commented Aug 6, 2018

WillAyd Aug 6, 2018

minggli Aug 6, 2018

WillAyd Aug 6, 2018

minggli Aug 6, 2018

WillAyd left a comment

jreback left a comment

jreback Aug 7, 2018

jreback Aug 7, 2018

jreback commented Aug 7, 2018

BUG: maximum recursion error when replacing empty lists #22083

BUG: maximum recursion error when replacing empty lists #22083

Conversation

minggli commented Jul 27, 2018 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minggli Jul 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minggli Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 28, 2018 • edited Loading

Codecov Report

minggli commented Jul 28, 2018 • edited Loading

Choose a reason for hiding this comment

minggli Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Aug 6, 2018 • edited Loading

Comment last updated on August 07, 2018 at 13:23 Hours UTC

minggli commented Aug 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Aug 7, 2018

minggli commented Jul 27, 2018 •

edited

Loading

minggli Jul 28, 2018 •

edited

Loading

minggli Aug 2, 2018 •

edited

Loading

codecov bot commented Jul 28, 2018 •

edited

Loading

minggli commented Jul 28, 2018 •

edited

Loading

minggli Aug 2, 2018 •

edited

Loading

pep8speaks commented Aug 6, 2018 •

edited

Loading