PERF: improves performance in GroupBy.cumcount
State: closed

closes #12839 

```
    -------------------------------------------------------------------------------
    Test name                                    | head[ms] | base[ms] |  ratio   |
    -------------------------------------------------------------------------------
    groupby_ngroups_10000_cumcount               |   3.9790 |  74.9559 |   0.0531 |
    groupby_ngroups_100_cumcount                 |   0.6043 |   0.9940 |   0.6079 |
    -------------------------------------------------------------------------------
    Test name                                    | head[ms] | base[ms] |  ratio   |
    -------------------------------------------------------------------------------

    Ratio < 1.0 means the target commit is faster then the baseline.
    Seed used: 1234

    Target [2a1c935] : PERF: improves performance in GroupBy.cumcount
    Base   [5b1f3b6] : reverts 'from .pandas_vb_common import *'
```


Labels:
name: Bug
name: Groupby
name: Performance
description: Memory or execution speed performance

Comments:
this would prob close #7569 
and partial on #5755 

pls add tests and/or advise.

ty

well i am not adding a new feature or closing a bug; so the tests already there should be fine

I think this crept in via #7910 / @mortada adding support for passing lists to nth.

@behzadnouri can you do a whatsnew note

Are we sure we want to change this? This has always been like that I think? (but indeed, inconsistent between SeriesGroupBy and DataFrameGroupBy ..)

@hayd this behaviour is already in 0.14.1 (while the list enhancement was only added in 0.15.0)

I think this should be changed as @behzadnouri suggest. Code is MUCH simpler. and everything would be consistent. I think what `nth` does now is a little odd (and IIRC is differently if you `.apply` it rather than directly use `.nth`)

This is really a bug-fix, but since is a fundamental change (e.g. a position based index vs the original index) being returned it ought to be highlited to the user. This just needs a simple Previous Behavior / New Behavior section with a code-block from the previous and actual from the new.

```
In [1]: df = DataFrame([[1, np.nan], [1, 4], [5, 6]], columns=['A', 'B'])
In [2]: g = df.groupby('A')
```

v0.16.2

```
In [6]: g.B.nth(0)
Out[6]: 
0   NaN
2     6
Name: B, dtype: float64

In [7]: g.B.nth(1)
Out[7]: 
1    4
Name: B, dtype: float64
```

This PR

```
In [4]: g.B.nth(0)
Out[4]: 
A
1   NaN
5     6
Name: B, dtype: float64

In [5]:  g.B.nth(1)
Out[5]: 
A
1    4
Name: B, dtype: float64
```

nth was supposed to be of the "filtering" type rather than aggregating type. Hence the v0.16.2 behaviour.

@hayd right! I forgot about that. so what should we do with all of this, the prior code had quite a number of 'hacks' to support that exact behavior.

moving to next version. This needs to not change the API. The result index can be set differently at the end.

api is the function signature which does not change. 

if you mean the behaviour, it is inconsistent between series and data-frame, and _even within series if there are nulls_; and seems to be a bug, not by design. so at some point you need to make series behave like frames or the other way around.

``` python
>>> df
   A  B
0  a  0
1  b  1
>>> df.groupby('A').nth(0)
   B
A   
a  0
b  1
>>> df.groupby('A')['B'].nth(0)
0    0
1    1
Name: B, dtype: int64

>>> ts
0     0
1   NaN
dtype: float64
>>> ts.groupby(Series(['a', 'b'])).nth(0, dropna=True)
a   NaN
b   NaN
dtype: float64
```

Series should behave like DataFrame (and filter rather than agg). I was _sure_ these worked the same on Series or it did behind a flag (which was not able to be the default due to old behaviour).

@hayd I think we are not using the same terminology :-)

Currently, nth DataFrame behaviour is _aggregating/reducing_ (so setting the grouping keys as the index), while Series behaviour is _filtering_ (keeping the original index labels) (so the other way around than I understand from your comment)

Originally `nth` on a DataFrame was filtering when you added it (https://github.com/pydata/pandas/pull/6569), but it was changed to reducing even before it was included in a release by @jreback  (https://github.com/pydata/pandas/pull/7044). Back then I asked specifically about this (https://github.com/pydata/pandas/pull/7044#issuecomment-42215735), but @jreback gave some reasons to make it reducing. Only, in that PR, I think it was an oversight of us that it only made DataFrame reducing, and left Series behaviour as filtering. 
Also in the docs, `nth` is mentioned in the reducing methods explicitely: the 'note box at the end of this section: http://pandas.pydata.org/pandas-docs/stable/groupby.html#aggregation

By the introduction of the possibility to pass lists to get multiple values, it complicated this a bit, as a real aggregating method will only return one value per group. Further, with the introduction of `dropna` kwarg, you cannot really call it filtering anymore when using that, since it will not return original rows always.

IIRC (and has been a while), we want to make `nth(0,dropna=True) == .first()` and `nth(0,dropna=True) == .last()`

@jreback and that is true for a DataFrame, but not Series (so this PR makes that analogy more consistent)

Now, for a solution. I think we just have to make a choice ..

1) Leave it as is (DataFrame as reducing, Series as filtering, it has been like this since 0.14)
2) Have all as reducing (so change Series)
3) Have all as filtering (and change DataFrame behaviour)

If we change something, I would go for 2), as the filtering behaviour is easy to get by using `as_index=False` (while for the other way around you have to do `set_index()`) + then it is more consistent with `first/last`

ahh, ok this was a consistency-fix partially then. I would agree with 2).

Some other inconsistencies:
- `as_index=False` is not working when using dropna:
  
  ```
  In [38]: df = pd.DataFrame({'a':list('ABABAB'), 'b':[1,2,3,4,5,6]})
  
  In [39]: df2 = df.copy()
  
  In [40]: df2.iloc[1,1] = np.nan
  
  In [41]: df2.groupby('a').nth(0, dropna='any')
  Out[41]:
  b
  a
  A  1
  B  4
  
  In [42]: df2.groupby('a', as_index=False).nth(0, dropna='any')
  Out[42]:
  b
  a
  A  1
  B  4
  
  In [43]: df2.groupby('a', as_index=False).nth(0)
  Out[43]:
  a   b
  0  A   1
  1  B NaN
  ```
- the resulting index when using `as_index=False` is different between `nth` and `first`:
  
  ```
  In [45]: df.index = df.index + 10
  
  In [46]: df.groupby('a').nth(0)
  Out[46]:
  b
  a
  A  1
  B  2
  
  In [47]: df.groupby('a').first()
  Out[47]:
  b
  a
  A  1
  B  2
  
  In [48]: df.groupby('a', as_index=False).first()
  Out[48]:
  a  b
  0  A  1
  1  B  2
  
  In [50]: df.groupby('a', as_index=False).nth(0)
  Out[50]:
  a  b
  10  A  1
  11  B  2
  ```

these are already noted in #5755 (though not so explicity)

But here it is possibly on purpose, as for `nth` this is a way to get filtering behaviour (keep original index), while reducing methods give you just a default 0, 1, .., n index when using `as_index=False` as you never can have the original index. That is the problem with `nth` being some mixture of a reducing and filtering method.

@behzadnouri can you rebase / update

further need to clarify any API changes / fixes here

@jorisvandenbossche I don't understand why the same argument couldn't apply to .head, isn't .nth is the same in whether it's filtering/reducing. :( Or is this claim due to dropping NaN, the _really_ slow path inside nth.

IME the nth, index behaviour is rarely what you want, you just want the nth line regardless of NaN. Maybe we should have an iloc method/accessor on groupby that would be more explicit (and always fast) and do just that.

@hayd if you wouldn't mind putting up some examples of what you think we should be doing for the various filter/reducers: `.head/nth/first/iloc` (and I agree `.iloc` would be a nice clear accessor, though obviously wouldn't skip `nan`, we did in fact had`.irow``, but deprecated recently)

yeah, iirc it was deprecated cos it was slow... it seems that the nth path (which skips nans) is similarly slow. Do people use this feature/is it useful? I guess if we had iloc, as the fast path, we (I) can just ignore nth as an R-throwback. :p

ok, so who wants to update, propose something here 2) seemed to be the most useful way to go, IOW, fix Series to be reducing (as DataFrame is now), and easy to get filtering with `as_index=False`

cc @behzadnouri 
@hayd 
@jorisvandenbossche 

can you rebase/update

@jreback rebased

@behzadnouri ok thanks. Can you put a whatsnew sub-section that shows the changes as a user would care about them, IOW an example (you can take from tests) showing what used to happend and the (correct) new way.

@jreback added an example to whatsnew showing the changes

@jreback moved to api changes section