Bug in loc raised Error when non-integer slice was given for MultiIndex #37707

phofl · 2020-11-09T01:05:41Z

closes Partial date string indexing doesn't work on MultiIndex #25165
closes BUG: MultiIndex.get_loc errors if get_loc on level returns a slice #24263
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

A slice return caused issues everywhere, when this function was used. So I unpacked it right in there.

…Index

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

jreback · 2020-11-09T02:57:07Z

pandas/core/indexes/multi.py

@@ -2702,7 +2702,10 @@ def _get_loc_single_level_index(self, level_index: Index, key: Hashable) -> int:
        if is_scalar(key) and isna(key):
            return -1
        else:
-            return level_index.get_loc(key)
+            loc = level_index.get_loc(key)
+            # if isinstance(loc, slice):


this is commented?

Shit, no I tried something out. This has to get back in. Will add it back momentarily

Commented it back in. At least my test failed previously :)

jreback · 2020-11-09T02:57:44Z

pandas/tests/indexing/multiindex/test_loc.py

@@ -598,3 +598,28 @@ def test_getitem_loc_commutability(multiindex_year_month_day_dataframe_random_da
    result = ser[2000, 5]
    expected = df.loc[2000, 5]["A"]
    tm.assert_series_equal(result, expected)
+
+
+def test_getitem_loc_datetime():


put partial in the name of the test, move to pandas/tests/indexing/test_partial.py

pep8speaks · 2020-11-09T11:09:17Z

Hello @phofl! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-24 18:33:22 UTC

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

phofl · 2020-11-09T21:38:52Z

fixing #24263 was quite tricky. Have to return the slice here, because the codes do not equal our start value. I thinkt there is no other way as checking for slice outside of the function

� Conflicts: � pandas/tests/indexing/multiindex/test_loc.py

jbrockmendel · 2020-11-12T02:33:36Z

doc/source/whatsnew/v1.2.0.rst

@@ -469,6 +469,7 @@ Indexing
 - Bug in :meth:`Index.where` incorrectly casting numeric values to strings (:issue:`37591`)
 - Bug in :meth:`Series.loc` and :meth:`DataFrame.loc` raises when numeric label was given for object :class:`Index` although label was in :class:`Index` (:issue:`26491`)
 - 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 :meth:`DataFrame.loc` raised ``TypeError`` when non integer interval was given to select values from :class:`MultiIndex` (:issue:`25165`, :issue:`24263`)


"non-integer :class:`Interval`"

Thx, changed

pandas/core/indexes/multi.py

pandas/tests/indexing/multiindex/test_loc.py

pandas/core/indexes/multi.py

jbrockmendel · 2020-11-13T05:22:32Z

@jreback im about to turn in, pls ping me on these indexing PRs to review in the AM

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

jbrockmendel · 2020-11-14T02:18:10Z

doc/source/whatsnew/v1.2.0.rst

@@ -475,6 +475,7 @@ Indexing
 - Bug in :meth:`Series.loc` and :meth:`DataFrame.loc` raises when numeric label was given for object :class:`Index` although label was in :class:`Index` (:issue:`26491`)
 - 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.loc` raised ``TypeError`` when non-integer :class:`Interval` was given to select values from :class:`MultiIndex` (:issue:`25165`, :issue:`24263`)


the tests dont seem to have anything to do with Intervals. Am i missing something?

No, you are right. Poor wording on my side. I meant a slice.

jbrockmendel · 2020-11-14T02:19:31Z

pandas/core/indexes/multi.py

@@ -3048,22 +3055,25 @@ def convert_indexer(start, stop, step, indexer=indexer, codes=level_codes):

        else:

-            code = self._get_loc_single_level_index(level_index, key)
+            idx = self._get_loc_single_level_index(level_index, key)


side-note: _get_loc_single_level_index looks sketchy. if level_index contains NA values, we should probably be doing get_loc and not returning -1.

Will look into this as a follow up

Have looked into this. Problem is, we are taking an index level from the MultiIndex, which does not contain the nans, so we will always get a KeyError, even if the original MultiIndex had nan in this level.

pandas/tests/indexing/multiindex/test_partial.py

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

phofl · 2020-11-14T21:24:41Z

Hm the test I fixed was wrong previously I think

jreback

small comments, pls merge master

jreback · 2020-11-15T17:23:26Z

pandas/core/indexes/multi.py

                end = start + section.searchsorted(idx, side="right")
                start = start + section.searchsorted(idx, side="left")
            else:
+                if isinstance(idx, slice):


can you make this an if/else and then an else case

umm sort of, L2688 should be an elif on L2687

Sorry, should be ok now

pandas/core/indexes/multi.py

jbrockmendel · 2020-11-16T19:30:52Z

doc/source/whatsnew/v1.2.0.rst

@@ -584,6 +584,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:`DataFrame.loc` raised ``TypeError`` when non-integer slice was given to select values from :class:`MultiIndex` (:issue:`25165`, :issue:`24263`)


i think i incorrectly said in another PR to use a participle when I meant a gerund: "raised" -> "raising"

Thx, changed

jreback · 2020-11-17T01:39:49Z

pandas/core/indexes/multi.py

                end = start + section.searchsorted(idx, side="right")
                start = start + section.searchsorted(idx, side="left")
            else:
+                if isinstance(idx, slice):


umm sort of, L2688 should be an elif on L2687

jreback · 2020-11-17T01:40:09Z

pandas/core/indexes/multi.py

+            else:
+                start = level_codes.searchsorted(idx, side="left")
+                end = level_codes.searchsorted(idx, side="right")
+            if start == end:


blank line here

jreback · 2020-11-17T01:41:10Z

pandas/tests/indexing/multiindex/test_partial.py

+        expected = expected.swaplevel(0, 1)
+
+        result = df2.loc[:, "2019-02":, :]
+        tm.assert_frame_equal(result, expected)


if any way to parameterize this would be great

Eval is the only option I can think of, but would want to avoid this. Don't know how to parametrize the changing position of : can be done otherwise

the changing position of :

do you mean "2019-02": vs :"2019-02"? Those are slice("2019-02", None) and slice(None, "2019-02"), respectively.

Does this take the same code path? Then this would work for sure.

Not perfect, but way better. Thanks very much

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

pandas/tests/indexes/multi/test_indexing.py

jreback · 2020-11-24T00:50:01Z

this looks fine, can you rebase and ping on green

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

phofl · 2020-11-24T21:19:22Z

@jreback green

jreback · 2020-11-24T22:00:03Z

thanks @phofl very nice

phofl added 2 commits November 9, 2020 02:03

Bug in loc raised Error when non integer interval was given for Multi…

c8df1c8

…Index

Adjust tests

9e62c50

phofl added Indexing Related to indexing on series/frames, not to indexes themselves MultiIndex labels Nov 9, 2020

phofl added 4 commits November 9, 2020 02:51

Comment it back in

8377a79

Run black

e6acaac

Rename test

37fe677

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

550ac88

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

jreback requested changes Nov 9, 2020

View reviewed changes

phofl changed the title ~~Bug in loc raised Error when non integer interval was given for MulIndex~~ Bug in loc raised Error when non integer interval was given for MultiIndex Nov 9, 2020

phofl added 5 commits November 9, 2020 22:06

Change new implementation

d38c9e7

Do slicing outside

72345a4

Add test

04056ae

Change issue number

41dbbcc

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

07f13f3

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

phofl added 6 commits November 9, 2020 23:14

Delete None

adaa27b

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

061468c

� Conflicts: � pandas/tests/indexing/multiindex/test_loc.py

Change whatsnew

d59e7f7

Add comment

d9562c4

Improve test

c382af0

Change test description

694e469

jbrockmendel reviewed Nov 12, 2020

View reviewed changes

pandas/core/indexes/multi.py Show resolved Hide resolved

jbrockmendel reviewed Nov 12, 2020

View reviewed changes

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

phofl changed the title ~~Bug in loc raised Error when non integer interval was given for MultiIndex~~ Bug in loc raised Error when non-integer Interval was given for MultiIndex Nov 12, 2020

jreback added this to the 1.2 milestone Nov 13, 2020

jreback requested changes Nov 13, 2020

View reviewed changes

pandas/core/indexes/multi.py Outdated Show resolved Hide resolved

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

d9a4054

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

jbrockmendel reviewed Nov 14, 2020

View reviewed changes

pandas/tests/indexing/multiindex/test_partial.py Outdated Show resolved Hide resolved

phofl changed the title ~~Bug in loc raised Error when non-integer Interval was given for MultiIndex~~ Bug in loc raised Error when non-integer slice was given for MultiIndex Nov 14, 2020

phofl added 4 commits November 14, 2020 12:21

Fix comments

c13da44

Add testcases

0a6cab7

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

4ffe46e

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

Fix bugs

3e9baa6

jreback requested changes Nov 15, 2020

View reviewed changes

phofl added 2 commits November 15, 2020 19:00

Adress review comments

c579c88

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

0be97cd

jbrockmendel reviewed Nov 16, 2020

View reviewed changes

Fix whatsnew

923610e

jreback requested changes Nov 17, 2020

View reviewed changes

phofl added 2 commits November 17, 2020 22:21

Fix blank line and if else

fdd170e

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

3f3c133

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

jbrockmendel reviewed Nov 17, 2020

View reviewed changes

pandas/tests/indexes/multi/test_indexing.py Show resolved Hide resolved

phofl added 2 commits November 17, 2020 23:59

Parametrize test

4c17aeb

Run black

a766f98

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

48424fb

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

This was referenced Nov 24, 2020

BUG: loc returning wrong elements for non-monotonic DatetimeIndex #38010

Merged

Bug in DataFrame.loc returning elements in wrong order when indexer is differently ordered than object #37992

Merged

jreback approved these changes Nov 24, 2020

View reviewed changes

jreback merged commit 1cc030e into pandas-dev:master Nov 24, 2020

phofl deleted the 25165 branch November 24, 2020 22:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug in loc raised Error when non-integer slice was given for MultiIndex #37707

Bug in loc raised Error when non-integer slice was given for MultiIndex #37707

phofl commented Nov 9, 2020 •

edited

Loading

jreback Nov 9, 2020

phofl Nov 9, 2020

phofl Nov 9, 2020

jreback Nov 9, 2020

phofl Nov 9, 2020

pep8speaks commented Nov 9, 2020 •

edited

Loading

phofl commented Nov 9, 2020

jbrockmendel Nov 12, 2020

phofl Nov 12, 2020

jbrockmendel commented Nov 13, 2020

jbrockmendel Nov 14, 2020

phofl Nov 14, 2020

jbrockmendel Nov 14, 2020

phofl Nov 14, 2020

phofl Nov 14, 2020

phofl commented Nov 14, 2020

jreback left a comment

jreback Nov 15, 2020

phofl Nov 15, 2020

jreback Nov 17, 2020

phofl Nov 17, 2020

jbrockmendel Nov 16, 2020

phofl Nov 16, 2020

jreback Nov 17, 2020

jreback Nov 17, 2020

phofl Nov 17, 2020

jreback Nov 17, 2020

phofl Nov 17, 2020

jbrockmendel Nov 17, 2020

phofl Nov 17, 2020

phofl Nov 17, 2020

jreback commented Nov 24, 2020

phofl commented Nov 24, 2020

jreback commented Nov 24, 2020

Bug in loc raised Error when non-integer slice was given for MultiIndex #37707

Bug in loc raised Error when non-integer slice was given for MultiIndex #37707

Conversation

phofl commented Nov 9, 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

Choose a reason for hiding this comment

pep8speaks commented Nov 9, 2020 • edited Loading

Comment last updated at 2020-11-24 18:33:22 UTC

phofl commented Nov 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel 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

phofl commented Nov 14, 2020

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

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

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 Nov 24, 2020

phofl commented Nov 24, 2020

jreback commented Nov 24, 2020

phofl commented Nov 9, 2020 •

edited

Loading

pep8speaks commented Nov 9, 2020 •

edited

Loading