BUG: Bug in loc raised ValueError when setting value via boolean list #37761

phofl · 2020-11-11T16:22:50Z

closes iloc boolean mask setitem #20438
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Not quite sure if conversion to numpy array is necessary, but is_dtype_bool does not accept lists

jbrockmendel · 2020-11-12T02:24:52Z

pandas/core/indexing.py

@@ -602,7 +603,7 @@ def _get_setitem_indexer(self, key):
        """
        Convert a potentially-label-based key into a positional indexer.
        """
-        if self.name == "loc":
+        if self.name == "loc" or is_bool_dtype(np.array(key)):


is it only lists we want here, or would com.is_bool_indexer(key) work?

That is better, changed it :)

jbrockmendel · 2020-11-12T02:26:46Z

pandas/tests/indexing/test_loc.py

@@ -1955,6 +1955,13 @@ def test_loc_setitem_dt64tz_values(self):
        result = s2["a"]
        assert result == expected

+    def test_loc_setitem_boolean_list(self):


lets put this in TestLocBooleanMask

can this be shared by DataFrame?

few words after the GH ref to the effect of "specifically list key, not arraylike"

Done, added a DataFrame part to the test, parametrization would be quite ugly here

jreback

also the original issue wa about iloc, can you test that as well (not sure if this is a different bug then)

jreback · 2020-11-13T04:57:50Z

pandas/tests/indexing/test_loc.py

@@ -1644,6 +1644,18 @@ def test_loc_setitem_mask_td64_series_value(self):
        assert expected == result
        tm.assert_frame_equal(df, df_copy)

+    def test_loc_setitem_boolean_list(self):


can you parameterize the rhs by array as well as list

ahh i see the comment, but still think can parameterize.

� Conflicts: � doc/source/whatsnew/v1.2.0.rst

phofl · 2020-11-13T12:12:49Z

cc @jreback Thanks very much, iloc actually was a different issue.

The validation in
https://github.com/pandas-dev/pandas/blob/b96665777c083516c9cdf21d25b7d9ccf70c81bd/pandas/core/indexers.py#L153:L157
did not allow boolean lists. We have two options to fix this for iloc. I converted a boolean list to an array in the indexing module, because I was not sure if this validation was done on purpose this way. Alternatively we could adjust the referenced validation to allow boolean lists too

jreback · 2020-11-13T13:06:31Z

pandas/tests/indexing/test_loc.py

+    def test_loc_setitem_boolean_list(self, func, rhs_func):
+        # GH#20438 testing specifically list key, not arraylike
+        ser = Series([0, 1, 2])
+        getattr(ser, func)[[True, False, True]] = rhs_func([5, 10])


can you also parameterize on the indexing type as well (e.g. list / array / pd.Index)

Index is not allowed for iloc, added the remaining ones

jreback · 2020-11-13T13:06:51Z

cc @jbrockmendel

� Conflicts: � doc/source/whatsnew/v1.2.0.rst

jbrockmendel · 2020-11-14T01:24:02Z

doc/source/whatsnew/v1.2.0.rst

@@ -476,6 +476,7 @@ Indexing
 - Bug in :meth:`DataFrame.loc` returned requested key plus missing values when ``loc`` was applied to single level from :class:`MultiIndex` (:issue:`27104`)
 - Bug in indexing on a :class:`Series` or :class:`DataFrame` with a :class:`CategoricalIndex` using a listlike indexer containing NA values (:issue:`37722`)
 - Bug in :meth:`DataFrame.xs` ignored ``droplevel=False`` for columns (:issue:`19056`)
+- Bug in :meth:`Series.loc` raised an Error when input was filtered with a boolean list and values to set were a list with lower dimension (:issue:`20438`)


is there a specific exception we can say instead of Error?

ValueError, added it

jbrockmendel · 2020-11-14T01:27:52Z

pandas/tests/indexing/test_loc.py

+    @pytest.mark.parametrize("indexing_func", [list, np.array])
+    @pytest.mark.parametrize("func", ["loc", "iloc"])
+    @pytest.mark.parametrize("rhs_func", [list, np.array])
+    def test_loc_setitem_boolean_list(self, func, rhs_func, indexing_func):


its only the *-iloc-list cases that fail on master. at its heart, is this an iloc bug or a loc bug?

jbrockmendel · 2020-11-14T01:28:14Z

pandas/tests/indexing/test_loc.py

+    def test_loc_setitem_boolean_list(self, func, rhs_func, indexing_func):
+        # GH#20438 testing specifically list key, not arraylike
+        ser = Series([0, 1, 2])
+        getattr(ser, func)[indexing_func([True, False, True])] = rhs_func([5, 10])


in the np.array case, do we care if the ndarray has bool vs object dtype?

Object dtype is killed by https://github.com/pandas-dev/pandas/blob/b96665777c083516c9cdf21d25b7d9ccf70c81bd/pandas/core/indexers.py#L153:L157 and raises a ValueError too.

jbrockmendel · 2020-11-14T01:31:07Z

pandas/core/indexing.py

@@ -602,7 +602,7 @@ def _get_setitem_indexer(self, key):
        """
        Convert a potentially-label-based key into a positional indexer.
        """
-        if self.name == "loc":
+        if self.name == "loc" or com.is_bool_indexer(key):


i haven't fully formed an opinion, but i think another alternative would be to properly handle boolean masks in Block.setitem.

You are right, loc is not failing, this means we do not need this or. Removed it. Must have mixed something up before

Lists can be handled in Block.setitem already, but arrays from dtype object don't work. Lists are killed by the validation mentioned above in pandas.core.indexers. If we let them through, setitem seems to be able to handle them in the iloc testcases

jbrockmendel · 2020-11-14T01:32:27Z

does this have a perf impact? IIRC is_bool_indexer isn't completely cheap

phofl · 2020-11-15T13:31:07Z

Ran indexing asvs. Seems a bit weird

before           after         ratio
     [50ae0bfc]       [b5dd4534]
     <master>         <20438>   
-         249±9ms          225±8ms     0.90  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
-        30.5±2ms       27.4±0.4ms     0.90  indexing.ChainIndexing.time_chained_indexing(None)
-         252±7ms          218±8ms     0.87  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
-         255±7ms          215±5ms     0.84  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
-         181±3ms          144±2ms     0.80  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-         185±4ms          147±2ms     0.79  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-         190±9ms          150±3ms     0.79  indexing.NumericSeriesIndexing.time_getitem_lists(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-        43.1±1μs       26.1±0.9μs     0.60  indexing.NumericSeriesIndexing.time_getitem_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-     1.86±0.04ms      1.11±0.04ms     0.60  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-     1.70±0.05ms         953±30μs     0.56  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-         308±8μs          166±6μs     0.54  indexing.NumericSeriesIndexing.time_getitem_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-        296±10μs          159±4μs     0.54  indexing.NumericSeriesIndexing.time_loc_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-        86.8±3μs         44.3±1μs     0.51  indexing.NumericSeriesIndexing.time_loc_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

jbrockmendel · 2020-11-19T22:53:49Z

couple comments, needs rebase otherwise LGTM

� Conflicts: � doc/source/whatsnew/v1.2.0.rst � pandas/tests/indexing/test_iloc.py

github-actions · 2020-12-20T00:15:20Z

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

� Conflicts: � doc/source/whatsnew/v1.2.0.rst � pandas/tests/indexing/test_iloc.py

phofl · 2020-12-21T20:02:00Z

ping @jbrockmendel ?

github-actions · 2021-01-21T00:47:12Z

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

jbrockmendel · 2021-01-28T02:58:40Z

It looks like in master the test case raises in check_setitem_lengths. would it make sense to patch at that point?

phofl · 2021-01-28T22:17:33Z

We could check for lists there too and count the True occurrences. Implemented it.

pandas/core/indexers.py

� Conflicts: � pandas/tests/indexing/test_iloc.py

jreback · 2021-05-24T18:04:47Z

@phofl can you rebase (i am not sure when it was before), and ping on green.

phofl · 2021-05-24T19:41:43Z

@jreback greenish, we have same number of failures as on master

simonjayhawkins · 2021-05-25T09:29:20Z

Thanks @phofl

…pandas-dev#37761)

phofl added 3 commits November 11, 2020 12:57

Fix listlike loc bug

e38dc1c

Add test

e606f33

Run black

8e02de7

phofl added the Indexing Related to indexing on series/frames, not to indexes themselves label Nov 11, 2020

phofl added 3 commits November 11, 2020 17:33

Fix imports

c8bacf5

Change if condition and move test

64c3dfc

Modify test

17c0cda

jbrockmendel reviewed Nov 12, 2020

View reviewed changes

jreback requested changes Nov 13, 2020

View reviewed changes

phofl added 3 commits November 13, 2020 12:51

Add parametrization

272f53b

Convert boolean list for iloc

e37d37c

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

9adcd05

� Conflicts: � doc/source/whatsnew/v1.2.0.rst

jreback requested changes Nov 13, 2020

View reviewed changes

phofl added 6 commits November 13, 2020 14:27

Parametrize test

0a45a73

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

29ec9b5

� Conflicts: � doc/source/whatsnew/v1.2.0.rst

Remove if condition part

83745b1

Change whatsnew

7db17ce

Move test

a53ae1b

Change order of condition

b5dd453

jbrockmendel reviewed Nov 14, 2020

View reviewed changes

phofl changed the title ~~[BUG]: Bug in loc raised error when setting value via boolean list~~ [BUG]: Bug in loc raised ValueError when setting value via boolean list Nov 14, 2020

phofl changed the title ~~[BUG]: Bug in loc raised ValueError when setting value via boolean list~~ BUG: Bug in loc raised ValueError when setting value via boolean list Nov 14, 2020

phofl added 2 commits November 19, 2020 23:55

Adress review

834c816

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

da88226

� Conflicts: � doc/source/whatsnew/v1.2.0.rst � pandas/tests/indexing/test_iloc.py

github-actions bot added the Stale label Dec 20, 2020

phofl added 2 commits December 21, 2020 20:59

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

1293109

� Conflicts: � doc/source/whatsnew/v1.2.0.rst � pandas/tests/indexing/test_iloc.py

Move whatsnew

99a9330

phofl removed the Stale label Dec 21, 2020

github-actions bot added the Stale label Jan 21, 2021

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

0f9f515

phofl added 2 commits January 28, 2021 23:01

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

08b0362

Move validation

b971750

Cast to array

15ad761

jreback requested changes May 2, 2021

View reviewed changes

pandas/core/indexers.py Show resolved Hide resolved

jreback added this to the 1.3 milestone May 2, 2021

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

c716b31

� Conflicts: � pandas/tests/indexing/test_iloc.py

simonjayhawkins removed the Stale label May 24, 2021

jreback approved these changes May 24, 2021

View reviewed changes

Merge branch 'master' of https://github.com/pandas-dev/pandas into 20438

c5c15c9

simonjayhawkins merged commit e4d0c1d into pandas-dev:master May 25, 2021

phofl deleted the 20438 branch May 25, 2021 09:34

TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021

BUG: Bug in loc raised ValueError when setting value via boolean list (…

0b78d08

…pandas-dev#37761)

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021

BUG: Bug in loc raised ValueError when setting value via boolean list (…

329e866

…pandas-dev#37761)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Bug in loc raised ValueError when setting value via boolean list #37761

BUG: Bug in loc raised ValueError when setting value via boolean list #37761

phofl commented Nov 11, 2020 •

edited

Loading

jbrockmendel Nov 12, 2020

phofl Nov 12, 2020

jbrockmendel Nov 12, 2020

phofl Nov 12, 2020

jreback left a comment

jreback Nov 13, 2020

jreback Nov 13, 2020

phofl Nov 13, 2020

phofl commented Nov 13, 2020 •

edited

Loading

jreback Nov 13, 2020

phofl Nov 13, 2020

jreback commented Nov 13, 2020

jbrockmendel Nov 14, 2020

phofl Nov 14, 2020

jbrockmendel Nov 14, 2020

phofl Nov 14, 2020

jbrockmendel Nov 14, 2020

phofl Nov 14, 2020

jbrockmendel Nov 14, 2020

phofl Nov 14, 2020 •

edited

Loading

phofl Nov 14, 2020 •

edited

Loading

jbrockmendel commented Nov 14, 2020

phofl commented Nov 15, 2020 •

edited

Loading

jbrockmendel commented Nov 19, 2020

github-actions bot commented Dec 20, 2020

phofl commented Dec 21, 2020

github-actions bot commented Jan 21, 2021

jbrockmendel commented Jan 28, 2021

phofl commented Jan 28, 2021 •

edited

Loading

jreback commented May 24, 2021

phofl commented May 24, 2021

simonjayhawkins commented May 25, 2021

BUG: Bug in loc raised ValueError when setting value via boolean list #37761

BUG: Bug in loc raised ValueError when setting value via boolean list #37761

Conversation

phofl commented Nov 11, 2020 • 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

jreback left a 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

phofl commented Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Nov 13, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phofl Nov 14, 2020 • edited Loading

Choose a reason for hiding this comment

phofl Nov 14, 2020 • edited Loading

Choose a reason for hiding this comment

jbrockmendel commented Nov 14, 2020

phofl commented Nov 15, 2020 • edited Loading

jbrockmendel commented Nov 19, 2020

github-actions bot commented Dec 20, 2020

phofl commented Dec 21, 2020

github-actions bot commented Jan 21, 2021

jbrockmendel commented Jan 28, 2021

phofl commented Jan 28, 2021 • edited Loading

jreback commented May 24, 2021

phofl commented May 24, 2021

simonjayhawkins commented May 25, 2021

phofl commented Nov 11, 2020 •

edited

Loading

phofl commented Nov 13, 2020 •

edited

Loading

phofl Nov 14, 2020 •

edited

Loading

phofl Nov 14, 2020 •

edited

Loading

phofl commented Nov 15, 2020 •

edited

Loading

phofl commented Jan 28, 2021 •

edited

Loading