-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Bug in iloc aligned objects #37728
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
Bug in iloc aligned objects #37728
Changes from 16 commits
9d11511
ca3f47b
f1306aa
c55cd62
ea41407
6bced6a
689a1ab
d870dd6
23dab4a
48e0d25
ea068ba
8ff2430
8a697f7
3bef35a
7375f22
e58fbc8
55e9403
7016977
b965eee
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 |
---|---|---|
|
@@ -681,7 +681,7 @@ def __setitem__(self, key, value): | |
self._has_valid_setitem_indexer(key) | ||
|
||
iloc = self if self.name == "iloc" else self.obj.iloc | ||
iloc._setitem_with_indexer(indexer, value) | ||
iloc._setitem_with_indexer(indexer, value, self.name) | ||
|
||
def _validate_key(self, key, axis: int): | ||
""" | ||
|
@@ -1517,7 +1517,7 @@ def _get_setitem_indexer(self, key): | |
|
||
# ------------------------------------------------------------------- | ||
|
||
def _setitem_with_indexer(self, indexer, value): | ||
def _setitem_with_indexer(self, indexer, value, name="iloc"): | ||
""" | ||
_setitem_with_indexer is for setting values on a Series/DataFrame | ||
using positional indexers. | ||
|
@@ -1593,7 +1593,7 @@ def _setitem_with_indexer(self, indexer, value): | |
new_indexer = convert_from_missing_indexer_tuple( | ||
indexer, self.obj.axes | ||
) | ||
self._setitem_with_indexer(new_indexer, value) | ||
self._setitem_with_indexer(new_indexer, value, name) | ||
|
||
return | ||
|
||
|
@@ -1618,17 +1618,17 @@ def _setitem_with_indexer(self, indexer, value): | |
indexer, missing = convert_missing_indexer(indexer) | ||
|
||
if missing: | ||
self._setitem_with_indexer_missing(indexer, value) | ||
self._setitem_with_indexer_missing(indexer, value, name) | ||
return | ||
|
||
# align and set the values | ||
if take_split_path: | ||
# We have to operate column-wise | ||
self._setitem_with_indexer_split_path(indexer, value) | ||
self._setitem_with_indexer_split_path(indexer, value, name) | ||
else: | ||
self._setitem_single_block(indexer, value) | ||
self._setitem_single_block(indexer, value, name) | ||
|
||
def _setitem_with_indexer_split_path(self, indexer, value): | ||
def _setitem_with_indexer_split_path(self, indexer, value, name): | ||
""" | ||
Setitem column-wise. | ||
""" | ||
|
@@ -1642,7 +1642,7 @@ def _setitem_with_indexer_split_path(self, indexer, value): | |
if isinstance(indexer[0], np.ndarray) and indexer[0].ndim > 2: | ||
raise ValueError(r"Cannot set values with ndim > 2") | ||
|
||
if isinstance(value, ABCSeries): | ||
if isinstance(value, ABCSeries) and name != "iloc": | ||
value = self._align_series(indexer, value) | ||
|
||
# Ensure we have something we can iterate over | ||
|
@@ -1657,7 +1657,7 @@ def _setitem_with_indexer_split_path(self, indexer, value): | |
if is_list_like_indexer(value) and getattr(value, "ndim", 1) > 0: | ||
|
||
if isinstance(value, ABCDataFrame): | ||
self._setitem_with_indexer_frame_value(indexer, value) | ||
self._setitem_with_indexer_frame_value(indexer, value, name) | ||
|
||
elif np.ndim(value) == 2: | ||
self._setitem_with_indexer_2d_value(indexer, value) | ||
|
@@ -1714,7 +1714,7 @@ def _setitem_with_indexer_2d_value(self, indexer, value): | |
# setting with a list, re-coerces | ||
self._setitem_single_column(loc, value[:, i].tolist(), pi) | ||
|
||
def _setitem_with_indexer_frame_value(self, indexer, value: "DataFrame"): | ||
def _setitem_with_indexer_frame_value(self, indexer, value: "DataFrame", name): | ||
ilocs = self._ensure_iterable_column_indexer(indexer[1]) | ||
|
||
sub_indexer = list(indexer) | ||
|
@@ -1724,7 +1724,12 @@ def _setitem_with_indexer_frame_value(self, indexer, value: "DataFrame"): | |
|
||
unique_cols = value.columns.is_unique | ||
|
||
if not unique_cols and value.columns.equals(self.obj.columns): | ||
if name == "iloc": | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for i, loc in enumerate(ilocs): | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
val = value.iloc[:, i] | ||
self._setitem_single_column(loc, val, pi) | ||
|
||
elif 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: | ||
|
@@ -1787,7 +1792,7 @@ def _setitem_single_column(self, loc: int, value, plane_indexer): | |
# reset the sliced object if unique | ||
self.obj._iset_item(loc, ser) | ||
|
||
def _setitem_single_block(self, indexer, value): | ||
def _setitem_single_block(self, indexer, value, name): | ||
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. type 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. Done |
||
""" | ||
_setitem_with_indexer for the case when we have a single Block. | ||
""" | ||
|
@@ -1815,14 +1820,13 @@ def _setitem_single_block(self, indexer, value): | |
return | ||
|
||
indexer = maybe_convert_ix(*indexer) | ||
|
||
if isinstance(value, (ABCSeries, dict)): | ||
if isinstance(value, ABCSeries) and name != "iloc" or isinstance(value, dict): | ||
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. for follow-up, can you put parens where appropriate so i dont have to think about whether the "and" gets evaluated before the "or" |
||
# TODO(EA): ExtensionBlock.setitem this causes issues with | ||
# setting for extensionarrays that store dicts. Need to decide | ||
# if it's worth supporting that. | ||
value = self._align_series(indexer, Series(value)) | ||
|
||
elif isinstance(value, ABCDataFrame): | ||
elif isinstance(value, ABCDataFrame) and name != "iloc": | ||
value = self._align_frame(indexer, value) | ||
|
||
# check for chained assignment | ||
|
@@ -1833,7 +1837,7 @@ def _setitem_single_block(self, indexer, value): | |
self.obj._mgr = self.obj._mgr.setitem(indexer=indexer, value=value) | ||
self.obj._maybe_update_cacher(clear=True) | ||
|
||
def _setitem_with_indexer_missing(self, indexer, value): | ||
def _setitem_with_indexer_missing(self, indexer, value, name): | ||
""" | ||
Insert new row(s) or column(s) into the Series or DataFrame. | ||
""" | ||
|
@@ -1854,7 +1858,7 @@ def _setitem_with_indexer_missing(self, indexer, value): | |
if index.is_unique: | ||
new_indexer = index.get_indexer([new_index[-1]]) | ||
if (new_indexer != -1).any(): | ||
return self._setitem_with_indexer(new_indexer, value) | ||
return self._setitem_with_indexer(new_indexer, value, name) | ||
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. double check me on this, but i think it might be the case that we only get here 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. Yep, you are right. Iloc raises when indexer is a dict. That is the only way we can get in there. Removed it |
||
|
||
# this preserves dtype of the value | ||
new_values = Series([value])._values | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -801,6 +801,39 @@ def test_iloc_setitem_empty_frame_raises_with_3d_ndarray(self): | |
with pytest.raises(ValueError, match=msg): | ||
obj.iloc[nd3] = 0 | ||
|
||
def test_iloc_assign_series_to_df_cell(self): | ||
# GH 37593 | ||
df = DataFrame(columns=["a"], index=[0]) | ||
df.iloc[0, 0] = Series([1, 2, 3]) | ||
expected = DataFrame({"a": [Series([1, 2, 3])]}, columns=["a"], index=[0]) | ||
tm.assert_frame_equal(df, expected) | ||
|
||
@pytest.mark.parametrize("klass", [list, np.array]) | ||
def test_iloc_setitem_bool_indexer(self, klass): | ||
# GH#36741 | ||
df = DataFrame({"flag": ["x", "y", "z"], "value": [1, 3, 4]}) | ||
indexer = klass([True, False, False]) | ||
df.iloc[indexer, 1] = df.iloc[indexer, 1] * 2 | ||
expected = DataFrame({"flag": ["x", "y", "z"], "value": [2, 3, 4]}) | ||
tm.assert_frame_equal(df, expected) | ||
|
||
@pytest.mark.parametrize("indexer", [[1], slice(1, 2)]) | ||
def test_setitem_iloc_pure_position_based(self, indexer): | ||
# GH#22046 | ||
df1 = DataFrame({"a2": [11, 12, 13], "b2": [14, 15, 16]}) | ||
df2 = DataFrame({"a": [1, 2, 3], "b": [4, 5, 6], "c": [7, 8, 9]}) | ||
df2.iloc[:, indexer] = df1.iloc[:, [0]] | ||
expected = DataFrame({"a": [1, 2, 3], "b": [11, 12, 13], "c": [7, 8, 9]}) | ||
tm.assert_frame_equal(df2, expected) | ||
|
||
def test_setitem_iloc_dictionary_value(self): | ||
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. nitpick for future: setitem_iloc -> iloc_setitem |
||
# GH#37728 | ||
df = DataFrame({"x": [1, 2], "y": [2, 2]}) | ||
rhs = dict(x=9, y=99) | ||
df.iloc[1] = rhs | ||
expected = DataFrame({"x": [1, 9], "y": [2, 99]}) | ||
tm.assert_frame_equal(df, expected) | ||
|
||
|
||
class TestILocErrors: | ||
# NB: this test should work for _any_ Series we can pass as | ||
|
@@ -966,3 +999,11 @@ def test_iloc(self): | |
def test_iloc_getitem_nonunique(self): | ||
ser = Series([0, 1, 2], index=[0, 1, 0]) | ||
assert ser.iloc[2] == 2 | ||
|
||
def test_setitem_iloc_pure_position_based(self): | ||
# GH#22046 | ||
ser1 = Series([1, 2, 3]) | ||
ser2 = Series([4, 5, 6], index=[1, 0, 2]) | ||
ser1.iloc[1:3] = ser2.iloc[1:3] | ||
expected = Series([1, 5, 6]) | ||
tm.assert_series_equal(ser1, expected) |
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.
can you type name here (and above)
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.
Done