ENH: pd.MultiIndex.get_loc(np.nan) #28919

proost · 2019-10-11T07:35:50Z

MultiIndex.get_loc could not find nan with values including missing
values as a input.

Background: In MultiIndex, missing value is denoted by -1 in codes and doesn't exist in self.levels

So, could not find NA value in self.levels.

Before PR xref #28783

closes pd.MultiIndex.get_loc(np.nan) #19132
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pandas/core/indexes/multi.py

pandas/tests/indexes/multi/test_indexing.py

jbrockmendel · 2019-11-19T23:40:59Z

pandas/tests/indexes/multi/test_indexing.py

+    assert idx.get_loc((np.nan, 1)) == expected
+
+
+def test_get_indexer_with_missing_value():


testing get_loc and get_indexer are good, but we dont expect users to see those directly. is there any user-facing behavior that this changes that should be tested?

can you respond to this, is there any user facing change aside from .get_loc itself, IOW does user facing indexing change?

@jreback
.get_slice_bound, __contains__,slice_indexer, slice_locs affect from this change.
I add test case too.

pandas/core/indexes/multi.py

pep8speaks · 2019-11-21T09:09:53Z

Hello @proost! 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-01-07 13:28:41 UTC

pandas/core/indexes/multi.py

jreback · 2019-12-02T01:22:31Z

pandas/tests/indexes/multi/test_indexing.py

+    assert idx.get_loc((np.nan, 1)) == expected
+
+
+def test_get_indexer_with_missing_value():


can you respond to this, is there any user facing change aside from .get_loc itself, IOW does user facing indexing change?

proost · 2019-12-03T04:11:02Z

@jbrockmendel @jreback
get_slice_bound, __contains__, slice_indexer, slice_locs changes from this change. In short, these functions can index NA value. I think, this is better than before. Because As we treat NA value like a non-NA value, we can index more accurately when NA value is in 'MultiIndex'

If,

idx = MultiIndex.from_arrays([[np.nan, "a", "b"], ["c", "d", "e"]])

case1 : "get_slice_bound"

idx.get_slice_bound(np.nan, side="left", kind="ix")
Before:   raise exception
After:  0

case2 : "slice_indexer"

idx.slice_indexer(start=np.nan, end="a")   
Before: raise exception
After: slice(0, 2, None)

case3: "slice_locs"

idx.slice_locs(start=np.nan, end=("a","d"))  
Before: raise exception
After: (0,2)

case4: __contains__

assert np.nan in idx
Before: raise exception
After: Not raise exception

problem is when this PR is merged, if __contains__ is true, but "isin" can be false. because "isin" not affect from this change. so if this PR is merged, need to change "isin" for a consistency.

jreback · 2019-12-10T14:23:03Z

problem is when this PR is merged, if contains is true, but "isin" can be false. because "isin" not affect from this change. so if this PR is merged, need to change "isin" for a consistency.

can you show what you mean here?

proost · 2019-12-11T06:53:42Z

@jreback

idx = MultiIndex.from_arrays([[np.nan, "a", "b"], ["c", "d", "e"]])
assert np.nan in idx

not raise exception. But

idx.isin([np.nan],level=0)
array([False, False, False])

__contains__ says NA value in level 0 index, But isin says doesn't.

So If this PR is merged, change isin for consistency might be good.

jreback · 2019-12-27T19:48:15Z

can you merge master and will look again

jreback · 2020-01-03T02:09:28Z

So If this PR is merged, change isin for consistency might be good.

ok this PR looks good. can you add in support for .isin() in this PR? I think it would make sense to merge these simultaneously (e.g. you can also build on this one if that is easier)

proost · 2020-01-04T08:57:55Z

@jreback
Okay, I change .isin and add tests

jreback

looks good. some comments, ping on green.

@toobaz @jbrockmendel @TomAugspurger if you'd care to have a look

pandas/tests/indexes/multi/test_indexing.py

pandas/core/indexes/multi.py

jbrockmendel · 2020-01-07T02:19:52Z

pandas/core/indexes/multi.py

-            else:
-                return np.lib.arraysetops.in1d(level_codes, sought_labels)
+                return np.zeros(len(levs), dtype=np.bool_)
+            return levs.isin(values)


the edits from L3408 down to here look like they are just nice cleanups independent of the rest of this PR. is that accurate?

@jbrockmendel
For NA values, someone fixes #30677, then more accurate. "Index.isin" has a bug nonetheless in terms of checking NA value can be possible, This is more accurate

jbrockmendel · 2020-01-07T02:20:35Z

pandas/tests/indexes/multi/test_contains.py

+    [
+        ([("b", np.nan)], np.array([False, False, True]), None,),
+        ([np.nan, "a"], np.array([True, True, False]), 0),
+        (["d", np.nan], np.array([False, True, True]), 1),


is the issue specific to np.nan, or are there other NA values worth testing?

@jbrockmendel
xref #30677. Yes. it is specific to np.nan. In "Index", np.nan , np.NaT, None are discernible not denoted by NA value. So if MultiIndex mixs with np.nan, np.NaT, None all together, result of ".isin" are different from what we know.

jreback · 2020-01-09T03:11:48Z

thanks @proost very nice!

proost changed the title ~~ENH: pd.MultiIndex.get_loc(np.nan) (#19132)~~ ENH: pd.MultiIndex.get_loc(np.nan) Oct 11, 2019

proost force-pushed the get_loc-nan branch from 435ff04 to 2085411 Compare October 11, 2019 10:35

jreback added Indexing Related to indexing on series/frames, not to indexes themselves MultiIndex labels Oct 11, 2019

jreback requested changes Oct 11, 2019

View reviewed changes

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

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

proost force-pushed the get_loc-nan branch from e08cdf5 to daa5d65 Compare November 1, 2019 04:05

proost requested a review from jreback November 1, 2019 05:30

proost force-pushed the get_loc-nan branch 2 times, most recently from 5da4a8d to 9cf08c1 Compare November 5, 2019 02:51

jbrockmendel reviewed Nov 19, 2019

View reviewed changes

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

jbrockmendel reviewed Nov 19, 2019

View reviewed changes

jreback requested changes Nov 20, 2019

View reviewed changes

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

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

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

proost force-pushed the get_loc-nan branch from 1ae595f to 25f40c8 Compare November 21, 2019 09:09

proost force-pushed the get_loc-nan branch 7 times, most recently from 14bba33 to a17ef45 Compare November 22, 2019 14:27

jreback requested changes Dec 2, 2019

View reviewed changes

proost force-pushed the get_loc-nan branch from a0df2b5 to 245d4bb Compare December 2, 2019 08:33

proost added a commit to proost/pandas that referenced this pull request Dec 28, 2019

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

f2bac75

proost force-pushed the get_loc-nan branch from babf3ff to f2bac75 Compare December 28, 2019 04:40

proost added a commit to proost/pandas that referenced this pull request Dec 28, 2019

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

34073c4

proost force-pushed the get_loc-nan branch from f2bac75 to 34073c4 Compare December 28, 2019 04:43

proost added a commit to proost/pandas that referenced this pull request Dec 28, 2019

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

9a82050

proost force-pushed the get_loc-nan branch from 34073c4 to 9a82050 Compare December 28, 2019 13:44

proost added a commit to proost/pandas that referenced this pull request Dec 30, 2019

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

ac00f80

proost force-pushed the get_loc-nan branch from 9a82050 to ac00f80 Compare December 30, 2019 03:25

jreback added this to the 1.0 milestone Jan 3, 2020

proost added a commit to proost/pandas that referenced this pull request Jan 4, 2020

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

6acf82e

proost force-pushed the get_loc-nan branch from bdbd2aa to 6acf82e Compare January 4, 2020 08:55

proost added a commit to proost/pandas that referenced this pull request Jan 4, 2020

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

ffcd4d9

proost force-pushed the get_loc-nan branch from e6384f9 to ffcd4d9 Compare January 4, 2020 09:01

jreback requested changes Jan 5, 2020

View reviewed changes

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

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

ENH: pd.MultiIndex.get_loc(np.nan) (pandas-dev#28919)

6951b04

proost force-pushed the get_loc-nan branch from f698d02 to 6951b04 Compare January 6, 2020 23:26

jbrockmendel reviewed Jan 7, 2020

View reviewed changes

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

jbrockmendel reviewed Jan 7, 2020

View reviewed changes

proost force-pushed the get_loc-nan branch from 7a6ab6e to 6951b04 Compare January 7, 2020 13:26

Merge branch 'master' into get_loc-nan

29621b2

jreback approved these changes Jan 9, 2020

View reviewed changes

jreback merged commit 0721841 into pandas-dev:master Jan 9, 2020

proost deleted the get_loc-nan branch January 9, 2020 10:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: pd.MultiIndex.get_loc(np.nan) #28919

ENH: pd.MultiIndex.get_loc(np.nan) #28919

proost commented Oct 11, 2019 •

edited

Loading

jbrockmendel Nov 19, 2019

jreback Dec 2, 2019

proost Dec 2, 2019

pep8speaks commented Nov 21, 2019 •

edited

Loading

jreback Dec 2, 2019

proost commented Dec 3, 2019 •

edited

Loading

jreback commented Dec 10, 2019

proost commented Dec 11, 2019

jreback commented Dec 27, 2019

jreback commented Jan 3, 2020

proost commented Jan 4, 2020

jreback left a comment

jbrockmendel Jan 7, 2020

proost Jan 7, 2020

jbrockmendel Jan 7, 2020

proost Jan 7, 2020

jreback commented Jan 9, 2020

		assert idx.get_loc((np.nan, 1)) == expected


		def test_get_indexer_with_missing_value():

ENH: pd.MultiIndex.get_loc(np.nan) #28919

ENH: pd.MultiIndex.get_loc(np.nan) #28919

Conversation

proost commented Oct 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Nov 21, 2019 • edited Loading

Comment last updated at 2020-01-07 13:28:41 UTC

Choose a reason for hiding this comment

proost commented Dec 3, 2019 • edited Loading

jreback commented Dec 10, 2019

proost commented Dec 11, 2019

jreback commented Dec 27, 2019

jreback commented Jan 3, 2020

proost commented Jan 4, 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

jreback commented Jan 9, 2020

proost commented Oct 11, 2019 •

edited

Loading

pep8speaks commented Nov 21, 2019 •

edited

Loading

proost commented Dec 3, 2019 •

edited

Loading