Skip to content

Backport PR #47581 on branch 1.4.x (BUG: DataFrame.loc not aligning rhs df for single block case) #47984

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 8, 2022
Merged

Backport PR #47581 on branch 1.4.x (BUG: DataFrame.loc not aligning rhs df for single block case) #47984

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
671a13c
Move whatsnew
phofl Jul 3, 2022
d32a495
Remove test
phofl Aug 5, 2022
2293852
Remove other change
phofl Aug 5, 2022
011d9c9
Remove new part of test
phofl Aug 5, 2022
b1531b7
Adress review
phofl Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.4.4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Fixed regressions
~~~~~~~~~~~~~~~~~
- Fixed regression in :func:`concat` materializing :class:`Index` during sorting even if :class:`Index` was already sorted (:issue:`47501`)
- Fixed regression in :meth:`DataFrame.loc` not updating the cache correctly after values were set (:issue:`47867`)
- Fixed regression in :meth:`DataFrame.loc` not aligning index in some cases when setting a :class:`DataFrame` (:issue:`47578`)
- Fixed regression in setting ``None`` or non-string value into a ``string``-dtype Series using a mask (:issue:`47628`)
-

Expand Down
7 changes: 5 additions & 2 deletions pandas/core/frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4527,8 +4527,11 @@ def _sanitize_column(self, value) -> ArrayLike:
"""
self._ensure_valid_index(value)

# We should never get here with DataFrame value
if isinstance(value, Series):
# We can get there through isetitem with a DataFrame
# or through loc single_block_path
if isinstance(value, DataFrame):
return _reindex_for_setitem(value, self.index)
elif isinstance(value, Series):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit weird now, but is closer to main than combining this two branches

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right so the comment # We should never get here with DataFrame value was not true?

the new comment refers to isetitem which is not on 1.4.x so slightly confusing.

it looks weird because could have just done if isinstance(value, (Series, DataFrame)): instead?

and the case of handling a dict-like value for the rhs was not working on 1.3.5 so we leaving that behavior unchanged here?

and slightly OT for the backport, on main we get a rogue warning?

FutureWarning: In a future version, `df.iloc[:, i] = newvals` will attempt to set the values inplace instead of always setting a new array. To retain the old behavior, use either `df[df.columns[i]] = newvals` or, if columns are non-unique, `df.isetitem(i, newvals)`
  result.loc[:, "var"] = rhs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right so the comment # We should never get here with DataFrame value was not true?

Yes, this was incorrect

the new comment refers to isetitem which is not on 1.4.x so slightly confusing.

good point forgot about this. will remove

it looks weird because could have just done if isinstance(value, (Series, DataFrame)): instead?

wanted to keep it closer to main, but can combine

and the case of handling a dict-like value for the rhs was not working on 1.3.5 so we leaving that behavior unchanged here?

correct

Yeah the warning is a bit annoying. Intend to look into this soonish

return _reindex_for_setitem(value, self.index)

if is_list_like(value):
Expand Down
9 changes: 9 additions & 0 deletions pandas/tests/frame/indexing/test_indexing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,15 @@ def test_loc_internals_not_updated_correctly(self):
)
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize("val", ["x", 1])
@pytest.mark.parametrize("idxr", ["a", ["a"]])
def test_loc_setitem_rhs_frame(self, idxr, val):
# GH#47578
df = DataFrame({"a": [1, 2]})
df.loc[:, "a"] = DataFrame({"a": [val, 11]}, index=[1, 2])
expected = DataFrame({"a": [np.nan, val]})
tm.assert_frame_equal(df, expected)


class TestDataFrameIndexingUInt64:
def test_setitem(self, uint64_frame):
Expand Down
7 changes: 7 additions & 0 deletions pandas/tests/frame/indexing/test_setitem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,13 @@ def test_setitem_npmatrix_2d(self):

tm.assert_frame_equal(df, expected)

def test_setitem_rhs_dataframe(self):
# GH#47578
df = DataFrame({"a": [1, 2]})
df["a"] = DataFrame({"a": [10, 11]}, index=[1, 2])
expected = DataFrame({"a": [np.nan, 10]})
tm.assert_frame_equal(df, expected)


class TestSetitemTZAwareValues:
@pytest.fixture
Expand Down