ENH: Add dropna in groupby to allow NaN in keys
State: closed

- [x] closes #3729 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Note that this PR will *NOT* fix the issue for `pivot_table` for now, the reason is that there is already an argument called `dropna` in `pivot_table` and it has slightly different meaning, currently it means: `Do not include columns whose entries are all NaN`.

I would propose a change in the follow-up PR for this since this is an api change: change the name of current `dropna` to `drop_all_na` maybe? and then add `dropna` to it and it is aligned with the `dropna` in groupby.


Summary:
After this PR, it will optional to inlcude NaN in group keys, e.g. below, and i also add example in docstring as well:
```python
a = [['a', 'b', 12, 12, 12], ['a', None, 12.3, 233., 12], ['b', 'a', 123.23, 123, 1], ['a', 'b', 1, 1, 1.]]
df = pd.DataFrame(a, columns=['a', 'b', 'c', 'd', 'e'])

df.groupby(by=['a', 'b']).sum()
```
will get
![Screen Shot 2020-01-01 at 11 01 16 AM](https://user-images.githubusercontent.com/9269816/71640222-19e68900-2c86-11ea-8749-c223729eec99.png)

with `dropna=False`,
```python
df.groupby(by=['a', 'b'], dropna=False).sum()
```
![Screen Shot 2020-01-01 at 11 01 23 AM](https://user-images.githubusercontent.com/9269816/71640225-2ff44980-2c86-11ea-9c66-a18147179c12.png)

For Series, it is the same:
```python
s = pd.Series([1, 2, 3, 3], index=["a", 'a', 'b', np.nan])

s.groupby(level=0).sum()
s.groupby(level=0, dropna=False).sum()
```
![Screen Shot 2020-01-01 at 11 01 32 AM](https://user-images.githubusercontent.com/9269816/71640228-4d291800-2c86-11ea-9d66-4a4be1a7347d.png)


Labels:
name: Groupby
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
Hi, @jreback 
I addressed all your comments, and merged master, pls let me know if you would like to see other changes.

Is this PR still active? I just ran into this while trying to implement a [small feature in dask](https://github.com/dask/dask/issues/5306) that this is blocking. Please let me know if you need any help @charlesdong1991 
@jsignell Thanks for pushing it, and i just updated the PR to address reviews. But I think it might not be close to merge, and could take time for discussions. 

@jreback I think I addressed most of your previous comments, except https://github.com/pandas-dev/pandas/pull/30584#discussion_r363406093 and this is the reason why the index type check has to be set to False, however, I do think the result of this is correct, and might need to think about a bit how MI should deal with NA, i opened the issue before #30750 and the pd.testing here constructs MI via `from_*` and will automatically drop NA in levels, and this lead to the `inferred_type` to `string`, and I think `mixed` should be correct input here, because we do include NA in levels, and thus `inferred_type` should be `mixed`. 

Wow that was a quick response @charlesdong1991! I'm not in any particular rush, just wanted to bump this back up in people's todo lists.
@WillAyd thanks for your reviews, and I had changes based on your reviews, feel free to give another look!
Hello @charlesdong1991! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

















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

##### Comment last updated at 2020-05-07 07:08:44 UTC
@WillAyd thanks for taking a look again!

I reduce the tests quite massively, and should look simpler! thanks for the suggestion!
Hi, @WillAyd @jreback 

I feel that we see the light in the end of tunnel for this PR 😄 quick summary to address all your comments:
1. Removed `check_index_type=False`! which i think was the biggest issue before, the reason it was there was when constructing MI for `expected`, if `NaN` in there, MI will automatically drop it from levels, so I need to manually set level for `expected` to include `NaN` if `dropna=False`
2. Rephrase/Reorg docs in `groupby.rst` and `1.1.0rst`, but I think there are still a couple comments about the `ref` is not clear to me, feel free to complement!
3. Simplified the tests significantly: basically compress all 3 similar tests for datetime-alike into one test with parametrization, and replace `na_values` in parametrization with existing `nulls_fixture` and `unique_nulls_fixture` from `conftest.py`
4. Remove xfail for windows machine builds in `test_algo.py`, the reason it was there was integer array has by default dtype of `int32` in windows i assume, so there is `dtype` conflict, therefore, I typed fixed dtype for it to avoid using xfail.

Feel free to take a look again, and hopefully lets get it merged soon!! ^^
any followups on this one? @jreback @WillAyd ^^
@jorisvandenbossche @TomAugspurger @jbrockmendel if you can have a quick skim.
ping @jreback while waiting for other reviews

p.s. somehow, there are less-than-usual CI checks after a rebase (i expect at least 13 or so, but see only 6, seems a lot of checks on other OS or python version are not triggered), not sure how/why, and pls let me know if another rebase is needed to see if the change is green on all.
> ping @jreback while waiting for other reviews
> 
> p.s. somehow, there are less-than-usual CI checks after a rebase (i expect at least 13 or so, but see only 6, seems a lot of checks on other OS or python version are not triggered), not sure how/why, and pls let me know if another rebase is needed to see if the change is green on all.

hmm, try rebasing again, this happened once before then resolved itself.
emm, weird, indeed seems it resolves itself. @jreback 
ping for reviews ^^

@jreback @jbrockmendel @TomAugspurger @jorisvandenbossche 
Don't wait around on me.

On Tue, Apr 28, 2020 at 5:21 AM Kaiqi Dong <notifications@github.com> wrote:

> ping for reviews ^^
>
> @jreback <https://github.com/jreback> @jbrockmendel
> <https://github.com/jbrockmendel> @TomAugspurger
> <https://github.com/TomAugspurger> @jorisvandenbossche
> <https://github.com/jorisvandenbossche>
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/30584#issuecomment-620517194>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIQFPAQ3UJ52HM43DWLRO2U4PANCNFSM4KBWP3MQ>
> .
>

maybe @WillAyd  @jbrockmendel could take a look for some feedbacks? 
@charlesdong1991 can you merge master and ping on green.
ping @jreback @WillAyd 
thanks @charlesdong1991 very nice! this has been a long time requested feature!
stupid question: how does that feature propagate into the standard pandas branches? any timeframe?
My understanding is this is slated for inclusion in pandas 1.1.0, but I'm not sure when that is planned for.