BUG: fix .loc.setitem not raising when using too many indexers #44656

DriesSchaumont · 2021-11-28T20:56:00Z

closes Series.loc/iloc[x, y] does not raise exception #13831
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

phofl · 2021-11-28T22:02:04Z

pandas/tests/indexing/test_loc.py

+    df = DataFrame({"a": [10]})
+    msg = "Too many indexers"
+    with pytest.raises(IndexingError, match=msg):
+        df["a"].loc[0, 0] = 1000


Can you construct the series explicitly?

jreback · 2021-11-28T23:56:44Z

pandas/tests/indexing/test_loc.py

@@ -2715,6 +2715,13 @@ def test_loc_getitem_multiindex_tuple_level():
    assert result2 == 6


+def test_loc_setitem_indexer_length():
+    df = DataFrame({"a": [10]})


can you cover all of the tests in the OP (parameterize if possible)

pandas/tests/indexing/test_loc.py

DriesSchaumont · 2021-11-30T14:02:37Z

pandas/tests/indexing/test_iloc.py

+    )
+    def test_iloc_setitem_indexer_length(self, ser, keys):
+        # GH#13831
+        with pytest.raises(IndexError, match="too many indices for array"):


Note the difference in error between ser.iloc[keys] = 1000 and ser.iloc[keys]. IndexError is the result of the fact that we let numpy handle this, while pandas raises IndexingError. I was wondering if this should be made consistent? Us catching and re-raising from numpy seems ugly.

cc @jbrockmendel

but @DriesSchaumont I think we would want to catch that case & re-raise appropriately

IIRC a while back we removed some checks (for perf) and let numpy exceptions surface.

Not knowing anything about the specific use case, in cases where we have a choice between raising IndexError vs IndexingError (i.e. either would be a reasonable choice) I'd rather raise IndexError.

jreback · 2021-12-01T01:08:59Z

@phofl @jbrockmendel good here? (ex the question)

jreback · 2021-12-05T15:42:01Z

cc @phofl @jbrockmendel if any comments

pandas/tests/indexing/test_loc.py

doc/source/whatsnew/v1.4.0.rst

phofl

small comments

pandas/core/indexing.py

jbrockmendel · 2021-12-05T18:19:29Z

pandas/tests/indexing/test_loc.py

+        ser.loc[keys] = 1000
+
+    with pytest.raises(IndexingError, match=msg):
+        ser.loc[keys]


can use indexer_sli fixture to share this test with the iloc one i think

Done, also moved the tests.

pandas/core/indexing.py

jreback · 2021-12-18T00:02:51Z

@phofl @jbrockmendel good here?

jbrockmendel · 2021-12-18T00:27:27Z

pandas/tests/indexing/test_indexing.py

+
+def test_ser_list_indexer_exceeds_dimensions(indexer_sli):
+    # GH#13831
+    if indexer_sli == tm.setitem:


could make an indexer_li fixture

jreback · 2021-12-24T16:42:34Z

@phofl @jbrockmendel ok here?

jreback · 2022-01-08T15:22:52Z

@DriesSchaumont can you merge master

pandas/tests/indexing/test_indexing.py

DriesSchaumont · 2022-01-09T11:08:29Z

Should I move this to 1.5?

jreback · 2022-01-10T13:11:39Z

doc/source/whatsnew/v1.4.0.rst

@@ -809,6 +809,7 @@ Indexing
 - Bug in :meth:`DataFrame.loc.__setitem__` changing dtype when indexer was completely ``False`` (:issue:`37550`)
 - Bug in :meth:`IntervalIndex.get_indexer_non_unique` returning boolean mask instead of array of integers for a non unique and non monotonic index (:issue:`44084`)
 - Bug in :meth:`IntervalIndex.get_indexer_non_unique` not handling targets of ``dtype`` 'object' with NaNs correctly (:issue:`44482`)
+- Bug in :meth:`Series.loc.__setitem__` and :meth:`Series.loc.__getitem__` not raising when using multiple keys without using a :class:`MultiIndex` (:issue:`13831`)


yeah let's move this to 1.5

Done, I moved whatsnew

jreback · 2022-01-16T17:40:24Z

@DriesSchaumont if you can merge master and address comments

DriesSchaumont · 2022-01-30T20:50:17Z

@jreback @jbrockmendel merged master, please let me know which/if questions still need answering.

jreback · 2022-01-30T23:59:07Z

thanks @DriesSchaumont very nice!

…andas-dev#44656)

phofl reviewed Nov 28, 2021

View reviewed changes

jreback added Error Reporting Incorrect or improved errors from pandas Indexing Related to indexing on series/frames, not to indexes themselves Bug labels Nov 28, 2021

jreback reviewed Nov 28, 2021

View reviewed changes

DriesSchaumont force-pushed the fix-13831 branch from bd23ab4 to abb0dbf Compare November 30, 2021 09:43

BUG: fix .loc.__setitem__ not raising when using too many indexers

ca37c9c

DriesSchaumont force-pushed the fix-13831 branch from abb0dbf to ca37c9c Compare November 30, 2021 10:18

DriesSchaumont added 2 commits November 30, 2021 12:12

Merge remote-tracking branch 'upstream/master' into fix-13831

c34acb2

Updates solution and added extra test

d886470

DriesSchaumont commented Nov 30, 2021

View reviewed changes

pandas/tests/indexing/test_loc.py Show resolved Hide resolved

DriesSchaumont commented Nov 30, 2021

View reviewed changes

Add whatsnew

5bae428

DriesSchaumont requested review from phofl and jreback November 30, 2021 16:04

jreback added this to the 1.4 milestone Dec 1, 2021

phofl reviewed Dec 5, 2021

View reviewed changes

pandas/tests/indexing/test_loc.py Outdated Show resolved Hide resolved

phofl reviewed Dec 5, 2021

View reviewed changes

doc/source/whatsnew/v1.4.0.rst Outdated Show resolved Hide resolved

phofl reviewed Dec 5, 2021

View reviewed changes

jbrockmendel reviewed Dec 5, 2021

View reviewed changes

pandas/core/indexing.py Show resolved Hide resolved

jbrockmendel reviewed Dec 5, 2021

View reviewed changes

DriesSchaumont added 2 commits December 8, 2021 12:12

Updates for review.

06eb7e4

Merge remote-tracking branch 'upstream/master' into fix-13831

453e3ba

DriesSchaumont requested review from jbrockmendel and phofl December 8, 2021 12:22

jreback requested changes Dec 8, 2021

View reviewed changes

pandas/core/indexing.py Show resolved Hide resolved

DriesSchaumont added 2 commits December 16, 2021 15:21

Merge remote-tracking branch 'upstream/master' into fix-13831

8c53bba

Add test for list indexer.

8e44463

jreback approved these changes Dec 18, 2021

View reviewed changes

jbrockmendel reviewed Dec 18, 2021

View reviewed changes

DriesSchaumont added 2 commits December 19, 2021 16:23

Merge remote-tracking branch 'upstream/master' into fix-13831

e5a768f

Add indexer_li fixture.

7880015

DriesSchaumont requested a review from jbrockmendel December 21, 2021 15:47

jbrockmendel reviewed Jan 9, 2022

View reviewed changes

pandas/tests/indexing/test_indexing.py Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into fix-13831

f83cfc8

Add comment to test.

b5c664d

jreback reviewed Jan 10, 2022

View reviewed changes

jreback modified the milestones: 1.4, 1.5 Jan 10, 2022

DriesSchaumont and others added 3 commits January 18, 2022 09:04

Move docs to 1.5

f7bc4a0

Merge remote-tracking branch 'upstream/main' into fix-13831

8daced5

Merge remote-tracking branch 'upstream/main' into fix-13831

3ebf788

jreback merged commit 275b187 into pandas-dev:main Jan 30, 2022

phofl pushed a commit to phofl/pandas that referenced this pull request Feb 14, 2022

BUG: fix .loc.__setitem__ not raising when using too many indexers (p…

de5e40b

…andas-dev#44656)

yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022

BUG: fix .loc.__setitem__ not raising when using too many indexers (p…

1f57d31

…andas-dev#44656)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: fix .loc.setitem not raising when using too many indexers #44656

BUG: fix .loc.setitem not raising when using too many indexers #44656

DriesSchaumont commented Nov 28, 2021 •

edited

Loading

phofl Nov 28, 2021

jreback Nov 28, 2021

DriesSchaumont Nov 30, 2021

jreback Dec 1, 2021

jbrockmendel Dec 1, 2021

jreback commented Dec 1, 2021

jreback commented Dec 5, 2021

phofl left a comment

jbrockmendel Dec 5, 2021

DriesSchaumont Dec 8, 2021

jreback commented Dec 18, 2021

jbrockmendel Dec 18, 2021

DriesSchaumont Dec 19, 2021

jreback commented Dec 24, 2021

jreback commented Jan 8, 2022

DriesSchaumont commented Jan 9, 2022

jreback Jan 10, 2022

DriesSchaumont Jan 18, 2022

jreback commented Jan 16, 2022

DriesSchaumont commented Jan 30, 2022

jreback commented Jan 30, 2022

BUG: fix .loc.__setitem__ not raising when using too many indexers #44656

BUG: fix .loc.__setitem__ not raising when using too many indexers #44656

Conversation

DriesSchaumont commented Nov 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 1, 2021

jreback commented Dec 5, 2021

phofl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 24, 2021

jreback commented Jan 8, 2022

DriesSchaumont commented Jan 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jan 16, 2022

DriesSchaumont commented Jan 30, 2022

jreback commented Jan 30, 2022

BUG: fix .loc.setitem not raising when using too many indexers #44656

BUG: fix .loc.setitem not raising when using too many indexers #44656

DriesSchaumont commented Nov 28, 2021 •

edited

Loading