Repository: pandas-dev/pandas
Pull request: BUG : ValueError in case on NaN value in groupby columns
Fixes GH24740

- [x] closes #24740 
- [x] closes #21151
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

**Before**
```python
In [5]: ds.groupby("b").groups                                                                                   
Out[5]: {'b': Int64Index([1], dtype='int64'), 'c': Int64Index([2], dtype='int64')}
In [6]: ds.groupby("b", observed=True).groups                                                                    
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-6-d76af2ea5948> in <module>
----> 1 ds.groupby("b", observed=True).groups

~/Softwares/pandas/pandas/core/groupby/groupby.py in groups(self)
    388         """
    389         self._assure_grouper()
--> 390         return self.grouper.groups
    391 

```
**After**
```python
In [5]: ds.groupby('b',observed=True).groups                                                                     
Out[5]: {'b': Int64Index([1], dtype='int64'), 'c': Int64Index([2], dtype='int64')}

```

**Better Example**
```python
In [6]:  df = pd.DataFrame({'cat': pd.Categorical(['a', 'c', 'a'], 
   ...:                        categories=['a', 'b', 'd', 'e', 'f']), 
   ...:                        'vals': [1, 2, 3]})                                                               

In [7]: df                                                                                                       
Out[7]: 
   cat  vals
0    a     1
1  NaN     2
2    a     3

In [8]: df.groupby('cat').groups                                                                                 
Out[8]: 
{'a': Int64Index([0, 2], dtype='int64'),
 'b': Int64Index([], dtype='int64'),
 'd': Int64Index([], dtype='int64'),
 'e': Int64Index([], dtype='int64'),
 'f': Int64Index([], dtype='int64')}

In [9]: df.groupby('cat',observed=True).groups                                                                   
Out[9]: {'a': Int64Index([0, 2], dtype='int64')}



```


Problem statement:
BUG: Wrong grouping of categoricals when observed=True
Setup:

```python
>>> s1 = pd.Categorical([np.nan, 'a', np.nan, 'a'], categories=['a', 'b'])
>>> s2 = pd.Series([1, 2, 3, 4])
>>> df = pd.DataFrame({'s1': s1, 's2': s2})
```

Comparing results with ``observed=False`` and ``observed=True``: 

```python
>>> df.groupby('s1').sum()  # ok
    s2
s1
a    6
b    0
>>> df.groupby('s1', observed=True).sum()
     s2
s1
NaN   6  # should not be shown
a     0  # should be 6
```

Notice the value are assigned wrongly. 

Also, notice that NaN is now a possible label.

If the first value is not a Nan, the assignment works fine (but Nan is still a possible label)

```python
>>> df[1:].groupby('s1', observed=True).sum()
     s2
s1
a     6
NaN   0
```

#### Problem description

The problem concerns when there are unobserved labels and the first value is Nan. If there are no unobserved values, everyting seems alright from my checks.

Nan should probably not be a possible label.

#### Expected Output

I would assume same output as when ``òbserved=False``, but without unobserved labels:

```python
>>> df.groupby('s1', observed=True).sum()
    s2
s1
a    6
```

#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit: None
python: 3.6.3.final.0
python-bits: 32
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 78 Stepping 3, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None

pandas: 0.23.0
pytest: 3.4.0
pip: 10.0.1
setuptools: 38.4.1
Cython: 0.26.1
numpy: 1.14.0
scipy: 1.0.0
pyarrow: None
xarray: None
IPython: 6.3.1
sphinx: 1.7.4
patsy: None
dateutil: 2.6.1
pytz: 2017.3
blosc: None
bottleneck: None
tables: 3.4.2
numexpr: 2.6.2
feather: None
matplotlib: 2.1.0
openpyxl: 2.4.8
xlrd: None
xlwt: None
xlsxwriter: None
lxml: 4.1.1
bs4: 4.6.0
html5lib: 1.0.1
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

Exception when grouping on a modified category
#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd

ds = pd.DataFrame({"a": [1, 2, 3], "b": ["a", "b", "c"]}) 
ds["b"] = ds["b"].astype("category")
ds['b'].cat.remove_categories(['a'], inplace=True)

# works
ds.groupby("b").groups

# gives ValueError: Categorial categories cannot be null 
ds.groupby("b", observed=True).groups

```
#### Problem description

I wonder if that exception should be raised there?
Using 0.24 RC1

Thanks to all working on pandas!

Hints from issue discussion:
it seems to be related to issue #21133
I've also tested the above `groupby()` followed by `first()` and it seems to produce some unexpected results.

Setup:
```python
>>> s1 = pd.Categorical([np.nan, 'a', np.nan, 'a'], categories=['a', 'b'])
>>> s2 = pd.Series([1,2,3,4])
>>> 
>>> df = pd.DataFrame({'s1':s1, 's2':s2})
>>> df
    s1  s2
0  NaN   1
1    a   2
2  NaN   3
3    a   4
```

Comparing results with `observed=False` and `observed=True`:
```python
>>> df.groupby('s1').first()
     s2
s1     
a   2.0
b   NaN

>>> df.groupby('s1', observed=True).first()
      s2
s1      
NaN  2.0
a    NaN
```

When `observed=True`, the results is somewhat strange.
* Incorrect `s2=NaN` for `s1=a`. I think should be `s2=2.0` ?
* `NaN` appearing in the `groupby(...).first()` result for `s1`? I would expect no `NaN` under `s1`?

#### Output of `pd.show_versions()`

<details>

INSTALLED VERSIONS
------------------
commit: None
python: 2.7.13.final.0
python-bits: 64
OS: Darwin
OS-release: 17.5.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_AU.UTF-8
LOCALE: None.None

pandas: 0.23.0
pytest: 3.3.1
pip: 9.0.1
setuptools: 18.2
Cython: 0.25.2
numpy: 1.14.0
scipy: 0.16.0
pyarrow: None
xarray: None
IPython: None
sphinx: None
patsy: None
dateutil: 2.7.3
pytz: 2014.4
blosc: None
bottleneck: 1.2.0
tables: 3.4.2
numexpr: 2.6.4
feather: None
matplotlib: 1.4.3
openpyxl: 1.8.6
xlrd: 0.9.3
xlwt: None
xlsxwriter: 0.9.6
lxml: 3.5.0
bs4: None
html5lib: None
sqlalchemy: 1.0.9
pymysql: None
psycopg2: 2.7.4 (dt dec pq3 ext lo64)
jinja2: 2.9.6
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

@phil65 Thanks for the report! 
Can you provide a more easily reproducible example? Ideally you would construct a small test dataframe with code instead of relying on an external package (eg for adding this test case in the test suite, we don't want to use an external package)
Sure. Done.
Thanks! 
I can confirm the error on master (existed on 0.23.4 already as well). It also seems that things like a reduction with `ds.groupby("b", observed=True).sum()` do work (not error). Although they include a NaN in the grouped keys, which doesn't seem to be correct behaviour (and certainly related to the error)
This is certainly related: https://github.com/pandas-dev/pandas/issues/21151

Commit message: minor_documentation_fix

Patch to review:
diff --git a/doc/source/whatsnew/v0.24.0.rst b/doc/source/whatsnew/v0.24.0.rst
index 3268575c7064d..785e7a4a0053f 100644
--- a/doc/source/whatsnew/v0.24.0.rst
+++ b/doc/source/whatsnew/v0.24.0.rst
@@ -1782,6 +1782,7 @@ Groupby/Resample/Rolling
 - Bug in :meth:`DataFrame.groupby` did not respect the ``observed`` argument when selecting a column and instead always used ``observed=False`` (:issue:`23970`)
 - Bug in :func:`pandas.core.groupby.SeriesGroupBy.pct_change` or :func:`pandas.core.groupby.DataFrameGroupBy.pct_change` would previously work across groups when calculating the percent change, where it now correctly works per group (:issue:`21200`, :issue:`21235`).
 - Bug preventing hash table creation with very large number (2^32) of rows (:issue:`22805`)
+- Bug in :meth: `pandas.core.groupby.groups` which casues ``ValueError`` if ``observed=True`` and ``nan`` is present in group column (:issue:`22805`).
 
 Reshaping
 ^^^^^^^^^
diff --git a/pandas/core/groupby/grouper.py b/pandas/core/groupby/grouper.py
index d8df227d4911a..16c7ea687237c 100644
--- a/pandas/core/groupby/grouper.py
+++ b/pandas/core/groupby/grouper.py
@@ -299,6 +299,7 @@ def __init__(self, index, grouper=None, obj=None, name=None, level=None,
                 self._labels = self.grouper.codes
                 if observed:
                     codes = algorithms.unique1d(self.grouper.codes)
+                    codes = codes[codes != -1]
                 else:
                     codes = np.arange(len(categories))
 
diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py
index 144b64025e1c0..199002cfc294c 100644
--- a/pandas/tests/groupby/test_categorical.py
+++ b/pandas/tests/groupby/test_categorical.py
@@ -420,6 +420,18 @@ def test_observed_groups(observed):
     tm.assert_dict_equal(result, expected)
 
 
+def test_observed_groups_with_nan(observed=True):
+    # GH 24740
+    df = pd.DataFrame({'cat': pd.Categorical(['a', 'c', 'a'],
+                       categories=['a', 'b', 'd', 'e', 'f']),
+                       'vals': [1, 2, 3]})
+
+    g = df.groupby('cat', observed=observed)
+    result = g.groups
+    expected = {'a': Index([0, 2], dtype='int64')}
+    tm.assert_dict_equal(result, expected)
+
+
 def test_datetime():
     # GH9049: ensure backward compatibility
     levels = pd.date_range('2014-01-01', periods=4)


Code review comments:
diff_hunk: @@ -420,6 +420,18 @@ def test_observed_groups(observed):
     tm.assert_dict_equal(result, expected)
 
 
+def test_observed_groups_with_nan(observed=True):
path: pandas/tests/groupby/test_categorical.py
text: @user1:
You don't need `observed` as a keyword argument here; can just pass the literal value in the `groupby` call