BUG: GroupBy.get_group raises ValueError when group key contains NaT
State: closed

Closes #6992. Made `GroupBy.get_group` works even if the key contains `NaT`.

NOTE: One issue is that `GroupBy.groups` returns incorrect key in numpy 1.6. This seems to be caused by  `_convert_grouper` uses `grouper.reindex(axis).values` to return value. This looks doesn't affect to main functionalities, but is there any expected result?

```
import pandas as pd
import numpy as np
>>> np.__version__
1.6.2

>>> df = pd.DataFrame({'values': np.random.randn(8), 
                   'dt': [np.nan, pd.Timestamp('2013-01-01'), np.nan, pd.Timestamp('2013-02-01'),
                          np.nan, pd.Timestamp('2013-02-01'), np.nan, pd.Timestamp('2013-01-01')]})
>>> grouped = df.groupby('dt')

>>> grouped.groups
{1970-01-16 48:00:00: [1, 7], 1970-01-16 24:00:00: [3, 5]}
```


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

Comments:
you can't have nan in the int groupby nor datetime

the cython routines are very strict about types 

so u don't need to check for a non existsnt type

furthermore the datetimes won't be passed directly but as a i8 dtype

I understand `nan` should not be included in values of such indexes, but can be passed as `to_groupby` argument.

Existing cython logic doesn't care about index instance, but check `nan` for the argument. I didn't change the core logic, and added to `Nat` handling condition. 

@sinhrks haven't really looked at this, but does this solve #7227 as well?

@jreback Unfortunately No. #7227 seems to be caused by `TimeGrouper`,  and the fix is only for normal `groupby`. I'll take a deeper look later.

In above example to group by datetime column, `labels` argument for `groupby_template` will be `DatetimeIndex`, not i8. So I understand all functions created by `groupby_template` should have the logic to exclude `NaT`

```
index = [0 1 2 3 4 5 6 7]
labels = <class 'pandas.tseries.index.DatetimeIndex'>
[NaT, ..., 2013-01-01]
Length: 8, Freq: None, Timezone: None
```

hmm...`DatetimeIndex` should never be passed to a cython function, ONLY i8. can you show me where that is the case? (you can pass as object dtype, but i8 is MUCH faster so always pass that way)

@jreback The behavior seems to be caused by `Grouping`.
https://github.com/pydata/pandas/blob/master/pandas/core/groupby.py#L1881-1884

`grouper` is converted to `datetime` in following lines.
https://github.com/pydata/pandas/blob/master/pandas/core/groupby.py#L1831-1838

@sinhrks that doesn't seem a problem, its when its actually passed to cython that needs conversion (to and from), can you investigate? 

@sinhrks can you rebase and address comments?

@jreback You mean to convert `to_groupby` (group key) just before passed to cython func, correct?
https://github.com/pydata/pandas/blob/master/pandas/core/index.py#L1330-1331

Conversion looks possible if `to_gorupby` is datetime, but it also can be an object dtype.
Thus I think cython funcs should handle object in this case?

I think u should convert to i8 if possible (and reverse after) - evn though I guess it's handled as object now 

#7560 does this conversion now (actually these are converted to float then coerced back).

So rebase after I merge that and let's take another look

can you show groupby benches after this change..thanks

Thanks, but #7560 doesn't convert group keys, correct? Thus, `_algos.groupby_xxx` functions are still receives objects as `labels` argument even after #7560.

I think dtype should be converted in `Index.groupby` before calling cython funcs. Is it ok to change `Index.groupby` only to accept ndarray, or is it categorized as public API?

Ahh, yes was thinking of something else.

`Index.groupby` is ONLY used by groupby IIRC, so its not really public (so ok to change).

OK, let me try. Maybe there is no side-effect, because currently `_algos.groupby_xxx` only accepts `ndarray` (thus the point which error occurrs being changed if something passed other than ndarray).

@jreback I made it work when group-key has datelike dtypes, is this what you mean? 

Have to consider object dtype case and better way to coercing dtype back.

can you rebase and revisit this...thxs

pushing if you get 2 this in next day or 2 lmk

@sinhrks revisit?

@sinhrks status on this?

@jreback Will take a look again, but I don't have good idea how to handle `object` columns. Convert to  `i8` only when possible (and leave `object` dtype)?

`object` should be ok (in that their is a an object ctyhon grouper). can you show me what the problem is?

@sinhrks status?

@sinhrks can we revive this? and pls add a tests for #9236 

@sinhrks can you revisit / update

Sorry, will work again.

@sinhrks ok, pls rebase when you have a chance

@sinhrks let's do this for 0.17.0

Yes, fixed to work on the latest master.