Skip to content

BUG: Series.setitem raising ValueError when setting Series with scalar indexer #39358

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 8 commits into from
Jan 28, 2021
Merged

BUG: Series.setitem raising ValueError when setting Series with scalar indexer #39358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
09cc7f4
BUG: Series.setitem raising ValueError when setting Series with scala…
phofl Jan 23, 2021
2cb7a7b
Handle non object dtype series
phofl Jan 23, 2021
78450b9
Remove brackets
phofl Jan 23, 2021
33845f4
Improve performance
phofl Jan 25, 2021
9e7a46a
Use is integer
phofl Jan 25, 2021
29ba005
Merge branch 'master' of https://github.com/pandas-dev/pandas into 38303
phofl Jan 25, 2021
38a66e9
Parametrize
phofl Jan 26, 2021
371f88f
Replace is_scalar
phofl Jan 27, 2021
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.3.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ Indexing
- Bug in :meth:`DataFrame.loc.__setitem__` raising ValueError when expanding unique column for :class:`DataFrame` with duplicate columns (:issue:`38521`)
- Bug in :meth:`DataFrame.iloc.__setitem__` and :meth:`DataFrame.loc.__setitem__` with mixed dtypes when setting with a dictionary value (:issue:`38335`)
- Bug in :meth:`DataFrame.__setitem__` not raising ``ValueError`` when right hand side is a :class:`DataFrame` with wrong number of columns (:issue:`38604`)
- Bug in :meth:`Series.__setitem__` raising ``ValueError`` when setting a :class:`Series` with a scalar indexer (:issue:`38303`)
- Bug in :meth:`DataFrame.loc` dropping levels of :class:`MultiIndex` when :class:`DataFrame` used as input has only one row (:issue:`10521`)
- Bug in :meth:`DataFrame.__getitem__` and :meth:`Series.__getitem__` always raising ``KeyError`` when slicing with existing strings an :class:`Index` with milliseconds (:issue:`33589`)
- Bug in setting ``timedelta64`` values into numeric :class:`Series` failing to cast to object dtype (:issue:`39086`)
Expand Down
12 changes: 11 additions & 1 deletion pandas/core/indexing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2023,7 +2023,17 @@ def ravel(i):
return ser._values.copy()
return ser.reindex(ax)._values

elif is_scalar(indexer):
elif is_integer(indexer) and self.ndim == 1:
Copy link
Member

Choose a reason for hiding this comment

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

can any of this be shared with the is_scalar branch on L2036? The _get_axis(1) there looks wrong, and it looks sketchy that indexer doesnt actually get used there

Copy link
Member Author

@phofl phofl Jan 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We could generalize the code, but this makes it quite hard to read. Have already tried this.

The second case is when setting a series for a row in a DataFrame while the first case is setting a series (with length one obviously) as a row of a series.

We do not need the indexer in the second case, since only aligning the series index with the dataframe columns

Copy link
Member

@jbrockmendel jbrockmendel Jan 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

makes sense, thanks.

can at least the is_scalar check below be tightened to is_integer? (not necessarily in this PR)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is certainly possible

if is_object_dtype(self.obj):
return ser
ax = self.obj._get_axis(0)

if ser.index.equals(ax):
return ser._values.copy()

return ser.reindex(ax)._values[indexer]

elif is_integer(indexer):
ax = self.obj._get_axis(1)

if ser.index.equals(ax):
Expand Down
18 changes: 18 additions & 0 deletions pandas/tests/series/indexing/test_setitem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,24 @@ def test_setitem_negative_out_of_bounds(self):
with pytest.raises(IndexError, match=msg):
ser[-11] = "foo"

@pytest.mark.parametrize("indexer", [tm.loc, tm.at])
@pytest.mark.parametrize("ser_index", [0, 1])
Copy link
Contributor

Choose a reason for hiding this comment

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

can you parameterize over tm.loc and tm.at (i assume that .iloc / .iat take a different path)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, did not know that. Yes they do not align

def test_setitem_series_object_dtype(self, indexer, ser_index):
# GH#38303
ser = Series([0, 0], dtype="object")
idxr = indexer(ser)
idxr[0] = Series([42], index=[ser_index])
expected = Series([Series([42], index=[ser_index]), 0], dtype="object")
tm.assert_series_equal(ser, expected)

@pytest.mark.parametrize("index, exp_value", [(0, 42.0), (1, np.nan)])
def test_setitem_series(self, index, exp_value):
# GH#38303
ser = Series([0, 0])
ser.loc[0] = Series([42], index=[index])
expected = Series([exp_value, 0])
tm.assert_series_equal(ser, expected)


class TestSetitemSlices:
def test_setitem_slice_float_raises(self, datetime_series):
Expand Down