Repository: pandas-dev/pandas
Pull request: BUG: additional keys in groupby indices when NAs are present
- [x] closes #9304
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

`lib.indices_fast` included logic to skip over null values (-1's), but did not include the case where the first index is null. Currently this function is only used in `sorting.get_indexer_dict` where the case of all null group_index is handled separately.

Problem statement:
additional keys in groupby indices when NAs are present
```
In [386]: h = pd.DataFrame({'a':[1,2,1,np.nan,1], 'b':[1,2,3,3,2], 'c':[2,3,1,4,2]})

In [387]: gh=h.groupby(['a', 'b'])

In [388]: gh.groups.keys()
Out[388]: [(1.0, 2), (nan, 3), (1.0, 3), (1.0, 1), (2.0, 2)]

In [389]: gh.indices.keys()
Out[389]: [(1.0, 2), (1.0, 3), (2.0, 3), (1.0, 1), (2.0, 2)]  # Incorrect
```

The tuple (2.0, 3) should not be here.
The problem goes away when there are no NAs

Hints from issue discussion:
`pandas.core.groupby:_get_indices_dict` seems causing the problem.

```
In [61]: label_list = [ping.labels for ping in gh.grouper.groupings]
In [76]: keys = [_values_from_object(ping.group_index) for ping in gh.grouper.groupings]
In [77]: keys
Out[77]: [array([ 1.,  2.]), array([1, 2, 3])]
In [78]: label_list
Out[78]: [array([ 0,  1,  0, -1,  0]), array([0, 1, 2, 2, 1])]
In [79]: _get_indices_dict(label_list, keys)
Out[79]:
{(1.0, 2): array([4]),
 (1.0, 3): array([2]),
 (2.0, 3): array([3]),
 (1.0, 1): array([0]),
 (2.0, 2): array([1])}
```

groupbying with a `nan` group (or in a level) is not really supported. In general these cause weird types of issues. I'll mark it as a bug though.

Commit message: Fix namespace in test

Patch to review:
diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst
index f66098633b45e..5e6d5c8c7466d 100644
--- a/doc/source/whatsnew/v1.3.0.rst
+++ b/doc/source/whatsnew/v1.3.0.rst
@@ -285,6 +285,7 @@ Groupby/resample/rolling
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Bug in :meth:`SeriesGroupBy.value_counts` where unobserved categories in a grouped categorical series were not tallied (:issue:`38672`)
+- Bug in :meth:`.GroupBy.indices` would contain non-existent indices when null values were present in the groupby keys (:issue:`9304`)
 -
 
 Reshaping
diff --git a/pandas/_libs/lib.pyx b/pandas/_libs/lib.pyx
index 4e451fc33b055..de1997249b631 100644
--- a/pandas/_libs/lib.pyx
+++ b/pandas/_libs/lib.pyx
@@ -891,9 +891,17 @@ def indices_fast(ndarray index, const int64_t[:] labels, list keys,
     if n == 0:
         return result
 
-    start = 0
-    cur = labels[0]
-    for i in range(1, n):
+    # Start at the first non-null entry
+    j = 0
+    for j in range(0, n):
+        if labels[j] != -1:
+            break
+    else:
+        raise ValueError("cannot handle all null values")
+    cur = labels[j]
+    start = j
+
+    for i in range(j+1, n):
         lab = labels[i]
 
         if lab != cur:
diff --git a/pandas/tests/groupby/test_missing.py b/pandas/tests/groupby/test_missing.py
index 56cf400258f0f..e2ca63d9ab922 100644
--- a/pandas/tests/groupby/test_missing.py
+++ b/pandas/tests/groupby/test_missing.py
@@ -126,3 +126,12 @@ def test_min_count(func, min_count, value):
     result = getattr(df.groupby("a"), func)(min_count=min_count)
     expected = DataFrame({"b": [value], "c": [np.nan]}, index=Index([1], name="a"))
     tm.assert_frame_equal(result, expected)
+
+
+def test_indicies_with_missing():
+    # GH 9304
+    df = DataFrame({"a": [1, 1, np.nan], "b": [2, 3, 4], "c": [5, 6, 7]})
+    g = df.groupby(["a", "b"])
+    result = g.indices
+    expected = {(1.0, 2): np.array([0]), (1.0, 3): np.array([1])}
+    assert result == expected


Code review comments:
diff_hunk: @@ -891,9 +891,17 @@ def indices_fast(ndarray index, const int64_t[:] labels, list keys,
     if n == 0:
         return result
 
-    start = 0
-    cur = labels[0]
-    for i in range(1, n):
+    # Start at the first non-null entry
+    j = 0
+    for j in range(0, n):
+        if labels[j] != -1:
+            break
+    else:
+        raise ValueError("cannot handle all null values")
path: pandas/_libs/lib.pyx
text: @user1:
hmm, just return result no?

@user1:
do any tests hit this?

@author:
Good call, returning result is much better than raising. Currently no tests can possibly hit this because of short-circuit logic in `get_indexer_dict` (I changed the comment around it, see the diff), I merely added it to be defensive. Doesn't seem advantageous to remove the short-circuit logic.

@author:
The return of an empty dictionary is hit by several tests however.