BUG: Fix issue with incorrect groupby handling of NaT
State: closed

closes #10590 

For groupby the time stamps gets converted to integervalue `tslib.iNaT`
which is -9223372036854775808. The aggregation is then done using this
value with incorrect result as a consequence. The solution proposed here
is to replace its value by np.nan in case it is a `datetime64[ns]`


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

Comments:
test would be nice 

I have tried to add a test and update to doc. I am not very sure I have done this correctly :)

I don't know what the next step here is? Am I supposed to do something now or am I waiting for someone to have a look at what I have done? There is a **This branch has conflicts that must be resolved** Can I see what the conflicts are and fix them?

@larvian rebase your branch on upstream master to resolve the conflicts

@maximilianr Thanks! I will try to figure out how to do that :)

lots of tips [here](http://pandas.pydata.org/pandas-docs/stable/contributing.html)

Also, can you add tests for `timedelta` which looks affected also?

```
df = pd.DataFrame({'IX': ['A', 'A'], 'num': [np.nan, 100],
                  'dt': [np.nan, datetime.datetime.now()],
                  'td':[np.nan, datetime.timedelta(days=1)]})
# OK
df.min()
# IX                              A
# dt    2015-07-25 07:41:47.871654
# num                           100
# td                1 days 00:00:00
# dtype: object

# NG
# df.groupby('IX').min()
#     dt  num  td
# IX             
# A  NaT  100 NaT
```

I _think_ you can use `com.is_datetime_or_timedelta_dtype` to check dtype.
- https://github.com/pydata/pandas/blob/master/pandas/core/common.py#L2517

@jreback Thanks!
@sinhrks Ok I will look at that too.

Hm... I can't get this rebase to work. It gives conflicts which seems to come back after I fix :(
It seems to get messier the more I try.
Perhaps it is easiest to create a new pandas clone and apply my changes there?
I have to leave it for another day.

We can help walk you through the git problems. Could be easier in the chat room: https://gitter.im/pydata/pandas

Thanks Tom!
I think I need it :)

Just curious, what's next with this PR?
For now is there anything else I should do?

can you rebase / update

Sure... rebase done. Queued for Travis CI

Rebased yesterday again and it passed Travis CI.
Today again I have "This branch has conflicts that must be resolved".
This seem to happen all the time due to changes to v0.17.0.txt .
It seems wasteful of Travis time to run CI just because updates due to changes to v0.17.0.txt.
How to avoid the constant conflicts due to changes in this file?
This time I try to insert my line above the last line.

to avoid conflicts, don't insert the line at the end of the notes, put it somewhere else above. This is why I have left lots of blank space (which gradually gets filled up).

ping when green.

This one is green now

thanks!

[this added line](https://github.com/pydata/pandas/pull/10625/files#diff-720d374f1a709d0075a1f0a02445cd65R1535) has broken cython aggregation. You cannot assign `nan` to an `int64` view of an array.

before this patch:

``` python
In [1]: from string import ascii_lowercase

In [2]: np.random.seed(2718281)

In [3]: n = 1 << 21

In [4]: dr = date_range('2015-08-30', periods=n // 10, freq='T')

In [5]: df = DataFrame({
   ...:         '1st':np.random.choice(list(ascii_lowercase), n),
   ...:         '2nd':np.random.randint(0, 5, n),
   ...:         '3rd':np.random.choice(dr, n)})

In [6]: df.loc[np.random.choice(n, n // 10), '3rd'] = np.nan

In [7]: gr = df.groupby(['1st', '2nd'])

In [8]: %timeit gr.count()
The slowest run took 21.22 times longer than the fastest. This could mean that an intermediate result is being cached 
1 loops, best of 3: 13.3 ms per loop

In [9]: %timeit gr.count()
100 loops, best of 3: 13.8 ms per loop

In [10]: pd.__version__
Out[10]: '0.16.2+521.g207efc2'
```

with this patch:

``` python
In [8]: %timeit gr.count()
1 loops, best of 3: 144 ms per loop

In [9]: %timeit gr.count()
10 loops, best of 3: 149 ms per loop

In [10]: pd.__version__
Out[10]: '0.16.2+522.g9c2d1a6'
```

yes that should be iNaT

odd that the tests didn't pick this up 

@jreback it is modifying values which are already `iNaT`:

```
values[values == tslib.iNaT] = np.nan
```

this should already be a float here

ahh I see it is raising, being cause and taking the slow path...ok.thanks for the catch

@behzadnouri @jreback Thanks for pointing out the problem.
Is it that the cython aggregation function accepts `int64` does not like the `nan` and raises exception?
Not sure how to deal with it. Should all the `NaT` be dropped instead before aggregating?
Are there some aggregations where the `NaT` are important?