-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: RecursionError when attempting to replace np.nan values (#45725) #45749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
f2ff20d
cb42c6b
3841991
5340552
01d0a08
a4fc217
a966f77
13687c6
588c908
7afd630
f82878a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -661,6 +661,15 @@ def test_replace_simple_nested_dict_with_nonexistent_value(self): | |
result = df.replace({"col": {-1: "-", 1: "a", 4: "b"}}) | ||
tm.assert_frame_equal(expected, result) | ||
|
||
@pytest.mark.parametrize("dtype", [object]) | ||
roib20 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@pytest.mark.parametrize("to_replace, value", [(np.nan, pd.NA), (np.nan, None)]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added, but had to change the tests to explicitly use |
||
def test_replace_numpy_nan(self, dtype, to_replace, value): | ||
# GH#45725 ensure numpy.nan can be replaced with pandas.NA or None | ||
df = DataFrame({"A": [to_replace]}, dtype=dtype) | ||
result = df.replace({to_replace: value}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also test usage with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes done. |
||
expected = DataFrame({"A": [value]}, dtype=dtype) | ||
tm.assert_frame_equal(result, expected) | ||
|
||
def test_replace_value_is_none(self, datetime_frame): | ||
orig_value = datetime_frame.iloc[0, 0] | ||
orig2 = datetime_frame.iloc[1, 0] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,16 @@ def test_replace_explicit_none(self): | |
assert expected.iloc[-1] is None | ||
tm.assert_series_equal(result, expected) | ||
|
||
@pytest.mark.parametrize("dtype", [object]) | ||
@pytest.mark.parametrize("to_replace, value", [(np.nan, pd.NA), (np.nan, None)]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comments as above. consider just using the null_fixture which has all the nans There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
def test_replace_numpy_nan(self, dtype, to_replace, value): | ||
# GH#45725 ensure numpy.nan can be replaced with pandas.NA or None | ||
ser = pd.Series([to_replace], dtype=dtype) | ||
result = ser.replace({to_replace: value}) | ||
expected = pd.Series([value], dtype=dtype) | ||
tm.assert_series_equal(result, expected) | ||
assert result.dtype == object | ||
|
||
def test_replace_noop_doesnt_downcast(self): | ||
# GH#44498 | ||
ser = pd.Series([None, None, pd.Timestamp("2021-12-16 17:31")], dtype=object) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if i read the original issue correctly this then this was working in 1.4 so no actual change (its only a change in master), if so delete this note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you're right. I removed the note from the changelog.