Skip to content

BUG: iloc.__setitem__ with DataFrame value, multiple blocks, non-unique columns #36337

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 2 commits into from
Sep 13, 2020
Merged

BUG: iloc.__setitem__ with DataFrame value, multiple blocks, non-unique columns #36337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68acb24
BUG: iloc.__setitem__ with DataFrame value, multiple blocks
jbrockmendel Sep 13, 2020
38f1672
remove leftover comment
jbrockmendel Sep 13, 2020
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
44 changes: 34 additions & 10 deletions pandas/core/indexing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1690,18 +1690,42 @@ def _setitem_with_indexer(self, indexer, value):
sub_indexer = list(indexer)
multiindex_indexer = isinstance(labels, ABCMultiIndex)
# TODO: we are implicitly assuming value.columns is unique
unique_cols = value.columns.is_unique

if not unique_cols and value.columns.equals(self.obj.columns):
# We assume we are already aligned, see
# test_iloc_setitem_frame_duplicate_columns_multiple_blocks
for loc in ilocs:
item = item_labels[loc]
if item in value:
sub_indexer[info_axis] = item
v = self._align_series(
tuple(sub_indexer),
value.iloc[:, loc],
multiindex_indexer,
)
else:
v = np.nan

for loc in ilocs:
item = item_labels[loc]
if item in value:
sub_indexer[info_axis] = item
v = self._align_series(
tuple(sub_indexer), value[item], multiindex_indexer
)
else:
v = np.nan
self._setitem_single_column(loc, v, pi)

self._setitem_single_column(loc, v, pi)
elif not unique_cols:
raise ValueError(
"Setting with non-unique columns is not allowed."
)

else:
for loc in ilocs:
item = item_labels[loc]
if item in value:
sub_indexer[info_axis] = item
v = self._align_series(
tuple(sub_indexer), value[item], multiindex_indexer
)
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the end goal here?

Copy link
Member Author

Choose a reason for hiding this comment

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

i dont understand the question. 1718-1728 is just an indented version of what we have in the status quo

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like you separated out the cases and duplicated the code. So is this making it easier to understand, later refactoring, ?

Copy link
Member Author

Choose a reason for hiding this comment

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

its similar but not identical. this chunk is aligning by label while the chunk above is aligning positionally.

yes, i do hope to refactor this method before too long

Copy link
Contributor

Choose a reason for hiding this comment

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

ok cool

yes, i do hope to refactor this method before too long

yeah these indexing routines defintely need some TLC

v = np.nan

self._setitem_single_column(loc, v, pi)

# we have an equal len ndarray/convertible to our labels
# hasattr first, to avoid coercing to ndarray without reason.
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/indexing/test_iloc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,20 @@ def test_iloc_setitem_dups(self):
df.iloc[[1, 0], [0, 1]] = df.iloc[[1, 0], [0, 1]].reset_index(drop=True)
tm.assert_frame_equal(df, expected)

def test_iloc_setitem_frame_duplicate_columns_multiple_blocks(self):
# Same as the "assign back to self" check in test_iloc_setitem_dups
# but on a DataFrame with multiple blocks
df = pd.DataFrame([[0, 1], [2, 3]], columns=["B", "B"])

df.iloc[:, 0] = df.iloc[:, 0].astype("f8")
assert len(df._mgr.blocks) == 2
expected = df.copy()

# assign back to self
df.iloc[[0, 1], [0, 1]] = df.iloc[[0, 1], [0, 1]]

tm.assert_frame_equal(df, expected)

# TODO: GH#27620 this test used to compare iloc against ix; check if this
# is redundant with another test comparing iloc against loc
def test_iloc_getitem_frame(self):
Expand Down