WIP: ENH: pivot/groupby index with nan
State: closed

I'm working on a solution for issue #3729.

I've identified where to work and put in place a first, crude solution. I need to do more tests.
- [ ] closes  #3729.
- [ ] tests added / passed
- [ ] passes `git diff upstream/master | flake8 --diff`
- [ ] whatsnew entry

xref #9941, #5456, #6992, #443

---

2016-08-28: here are the tests I think need to be added / copied /adapted:

Basic tests:
- [x] Create a new test class, in a new file
- [ ] test_basic_with_nan with dtypes in object, datetimes, timedelta & datetime64 with tz
- [x] test_groupby_series_dropna
- [x] test_groupby_frame_dropna
- [ ] test_groupby_panel_dropna
- [x] test_groupby_multi_dropna

Tests with data types
- [x] integer, floats
- [x] datetime, timedelta
- [x] strings and other objects
- [x] None

Existing tests to copy or adapt from TestGroupBy:
- [x] test_first_last_nth
- [x] test_nth
- [x] test_nth_multi_index_as_expected
- [x] test_nth_multi_index
- [ ] test_group_selection_cache
- [x] test_basic
- [x] test_with_na
- [ ] test_series_groupby_nunique
- [ ] test_groupby_level_with_nas
- [ ] test_groupby_nat_exclude
- [ ] test_groupby_groups_datetimeindex
- [ ] test_nlargest
- [ ] test_nsmallest
- [ ] test_groupby_as_index_agg
- [ ] test_agg_api
- [ ] test_transform
- [ ] test_builtins_apply
- [ ] test_groupby_whitelist
- [x] test__cython_agg_general
- [x] test_ops_general
- [x] test_groupby_transform_with_nan_group

And also I will need to edit the documentation, for instance this part:
- [ ] Edit http://pandas-docs.github.io/pandas-docs-travis/groupby.html


Labels:
name: Groupby
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
Tests I still need to add:
- [ ] datetimes with NaT
- [ ] panel

@nbonnotte looking good. I think this needs quite a few more tests. This is actually a major change and needs some thorough testing. Essentially need to replicate lots of tests, just now with a NA group.

can you rebase / update

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=h1) Report
> Merging [#12607](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/1a117fc52bd07bba3e42121cf900d16c7184e622?src=pr&el=desc) will **increase** coverage by `0.04%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/12607/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #12607      +/-   ##
==========================================
+ Coverage   90.99%   91.04%   +0.04%     
==========================================
  Files         153      137      -16     
  Lines       50469    49312    -1157     
==========================================
- Hits        45926    44894    -1032     
+ Misses       4543     4418     -125
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `?` | |
| #single | `?` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `96.25% <ø> (-0.04%)` | :arrow_down: |
| [pandas/core/groupby.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS5weQ==) | `95% <100%> (+2.97%)` | :arrow_up: |
| [pandas/core/algorithms.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWxnb3JpdGhtcy5weQ==) | `94.48% <100%> (+0.02%)` | :arrow_up: |
| [pandas/tools/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL3Rvb2xzL3Bsb3R0aW5nLnB5) | `71.79% <0%> (-28.21%)` | :arrow_down: |
| [pandas/io/pickle.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL2lvL3BpY2tsZS5weQ==) | `74.28% <0%> (-5.26%)` | :arrow_down: |
| [pandas/conftest.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL2NvbmZ0ZXN0LnB5) | `91.66% <0%> (-3.79%)` | :arrow_down: |
| [pandas/util/\_tester.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX3Rlc3Rlci5weQ==) | `35.29% <0%> (-3.6%)` | :arrow_down: |
| [pandas/util/decorators.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvZGVjb3JhdG9ycy5weQ==) | `62.26% <0%> (-3.05%)` | :arrow_down: |
| [pandas/tools/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL3Rvb2xzL21lcmdlLnB5) | `91.78% <0%> (-1.91%)` | :arrow_down: |
| [pandas/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree#diff-cGFuZGFzL19faW5pdF9fLnB5) | `91.66% <0%> (-1.52%)` | :arrow_down: |
| ... and [152 more](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=footer). Last update [1a117fc...f7a4c60](https://codecov.io/gh/pandas-dev/pandas/pull/12607?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@jreback Well, this is a lot more tricky than I thought. I'll need to touch to some cython function

For instance, in the [`last` aggregation method](https://github.com/pydata/pandas/blob/6d8c04ce64d975c2fd8c902c0e5df343805bd112/pandas/src/generate_code.py#L764-767), some NaN values are forgotten. If the last two lines of a group are 

```
[[1,   2]
 [NaN, 3]]
```

we obtain `[1, 3]`, because the NaN value is not considered.

The thing is, I'm now sailing into what is for me uncharted territory.

Do you remember why it is necessary to check for NaN values?

EDIT: this is in fact a completely separate issue (#8427), which has nothing to do with the matter. I tumbled on this while adapting the existing tests.

can you rebase and i'll have a look

@jreback Done it

I've written a todo list with all the tests I think I might need to add / copy / adapt. I've put it in the first message, I'll be easier to find there I think.

If I'm missing something, please let me know.

The tests fail on Windows because of the check on dtype:

```
FAIL: test_groupby_series_dropna (pandas.tests.test_groupby.TestGroupBy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python27_64\envs\pandas\lib\site-packages\pandas\tests\test_groupby.py", line 794, in test_groupby_series_dropna
    assert_series_equal(result, expected)
  File "C:\Python27_64\envs\pandas\lib\site-packages\pandas\util\testing.py", line 1154, in assert_series_equal
    assert_attr_equal('dtype', left, right)
  File "C:\Python27_64\envs\pandas\lib\site-packages\pandas\util\testing.py", line 878, in assert_attr_equal
    left_attr, right_attr)
  File "C:\Python27_64\envs\pandas\lib\site-packages\pandas\util\testing.py", line 1018, in raise_assert_detail
    raise AssertionError(msg)
AssertionError: Attributes are different
Attribute "dtype" are different
[left]:  int64
[right]: int32
```

In the test, I had forced `dtype=int`, which means `int32` on Windows 64b (?)

I'm going to use `assert_stuff_equal` with `check_dtype=False`.

the changes for windows dtype comparison might signify and underlying problem
don't change the comparisons unless the expected is being contructed with a ndarray 
and pls note these test

@jreback I've removed the `check_dtype=False`, and the `dtype=int` I had put (I don't even remember why) which created the problem in the first place

so need to expand the testing to include object (e.g. strings), and datetimelike (datetimes, timedelta, period) with nans.

I think easiest is to create a new class in the Tester, e.g. TestGroupbyDropna and have the appropriate tests there (and name them the same as in TestGroupby). 

Alternative, you can move sections of tests to a separate testing class, e.g.


TestGroupbyNth

TestGroupbyAgg

TestGroupbyTransform

or even better is to reorge like other things

tests/groupby/nth.py, agg.py, transform.py, basic.py

etc.

So. could move the tests first (to a sub-dir), or put them in separate classes for now
can you rebase and we'll see where this is
I've rebased. 

Why does Appveyor say I've cancelled the tests?
thanks @nbonnotte 

appveyor is giving some troubles, they just fixed it, so requeing some things.
There is a problem when there is `None` in the grouping values. As it is, `None` is treated differently from `NaN`, and then there is an issue during sorting. 

I guess the correct solution would be to convert all `None` to `NaN`... Let's see how I can do that
@nbonnotte we already do inference for things like this on ``Grouping``, for datetimelike (e.g. things are set as NaT). So you will need to replace in object series None->np.nan. But this should be a very minimal type of check.
I have a problem with my MultiIndexes:
```
(Pdb) left.index
MultiIndex(levels=[[u'e', nan, u'c', u'y', u's', u'w', u'z', u'u', u'n', u't', u'b', u'm', u'o', u'g', u'p', u'j', u'q', u'a', u'v', u'x', u'k', u'f', u'r', u'h', u'i', u'l'], [2015-08-24 00:00:00, 2015-08-30 00:00:00, 2015-09-01 00:00:00, NaT, 2015-08-31 00:00:00, 2015-08-27 00:00:00, 2015-08-29 00:00:00, 2015-08-26 00:00:00, 2015-08-25 00:00:00, 2015-08-28 00:00:00, 2015-08-23 00:00:00]],
           labels=[[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 2, 7, 4, 11, 3, 8, 1, 12, 9, 13, 14, 15, 16, 17, 13, 18, 5, 11, 15, 6, 17, 1, 19, 7, 14, 20, 15, 17, 18, 3, 17, 18, 19, 21, 21, 12, 1, 12, 22, 20, 0, 23, 8, 5, 5, 21, 4, 23, 6, 23, 20, 9, 24, 23, 6, 22, 12, 4, 23, 19, 10, 25, 0, 17, 12, 10, 10, 22, 0], [0, 1, 2, 3, 0, 4, 5, 0, 6, 7, 7, 8, 7, 4, 5, 4, 9, 5, 4, 0, 4, 7, 9, 10, 9, 6, 7, 7, 1, 0, 4, 2, 7, 8, 9, 1, 4, 3, 10, 8, 9, 4, 10, 7, 5, 1, 2, 10, 5, 4, 9, 6, 6, 1, 5, 0, 8, 8, 0, 2, 5, 10, 2, 0, 8, 3, 8, 1, 9, 7, 1, 2, 10, 4, 0, 6, 9, 5, 7, 10]],
           names=[u'jim', u'joe'])

(Pdb) right.index
MultiIndex(levels=[[u'a', u'b', u'c', u'e', u'f', u'g', u'h', u'i', u'j', u'k', u'l', u'm', u'n', u'o', u'p', u'q', u'r', u's', u't', u'u', u'v', u'w', u'x', u'y', u'z'], [2015-08-23 00:00:00, 2015-08-24 00:00:00, 2015-08-25 00:00:00, 2015-08-26 00:00:00, 2015-08-27 00:00:00, 2015-08-28 00:00:00, 2015-08-29 00:00:00, 2015-08-30 00:00:00, 2015-08-31 00:00:00, 2015-09-01 00:00:00]],
           labels=[[3, -1, 2, 23, 17, 21, 24, 19, 12, 18, 1, 2, 19, 17, 11, 23, 12, -1, 13, 18, 5, 14, 8, 15, 0, 5, 20, 21, 11, 8, 24, 0, -1, 22, 19, 14, 9, 8, 0, 20, 23, 0, 20, 22, 4, 4, 13, -1, 13, 16, 9, 3, 6, 12, 21, 21, 4, 17, 6, 24, 6, 9, 18, 7, 6, 24, 16, 13, 17, 6, 22, 1, 10, 3, 0, 13, 1, 1, 16, 3], [1, 7, 9, -1, 1, 8, 4, 1, 6, 3, 3, 2, 3, 8, 4, 8, 5, 4, 8, 1, 8, 3, 5, 0, 5, 6, 3, 3, 7, 1, 8, 9, 3, 2, 5, 7, 8, -1, 0, 2, 5, 8, 0, 3, 4, 7, 9, 0, 4, 8, 5, 6, 6, 7, 4, 1, 2, 2, 1, 9, 4, 0, 9, 1, 2, -1, 2, 7, 5, 3, 7, 9, 0, 8, 1, 6, 5, 4, 3, 0]],
           names=[u'jim', u'joe'])
```

So `left` has explicit NaNs values, and not `right`, so we get
```
(Pdb) left.index.levels[0].inferred_type
'mixed'

(Pdb) right.index.levels[0].inferred_type
'string'
```

@jreback Should I alter the test to make the multiindexes comparable? If yes, how? Or is there a problem with my code that I should fix instead?
@nbonnotte having nans in an MultiIndex levels is an anti-pattern. How did you construct this?
@nbonnotte how's this coming
I'm not doing much, really. I think it's just that the groupings' labels aren't supposed to be NaNs. I need to fix this, obviously
good ideas, but needs rebase / update. pls comment if you wish to proceed.
Frankly, it's the number of tests to add or update that killed me.
@nbonnotte I'd like to take this up if you are not working on this. Can you confirm?
@TrigonaMinima No, I'm no longer working on it :)
I see this as a major drawback of the powerful groupby functions in pandas and the first time I came upon it, without any error messages, the return of an empty DataFrame really threw me off. I hope the previous work can be finished.