BUG in DataFrameGroupBy.rank raising an obscure TypeError
State: closed

Fix issue #11759

Two things:
1. `Series.rank` and `DataFrame.rank` have been moved to `generic.py` and get the same signature [done with PR #11924]
2. It can happen that aggregate functions are applied to grouped object in a recursive way. Instead of raising a plain `ValueError` when that fails, and catching it afterwards indistinctly, I propose to raise an `_ErrorContainer` exception containing the last meaningful error to have been raised. Thus, when everything else has failed, we can still recover a meaningful exception and throw it back at the user.


Labels:
name: Groupby
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
@jreback a test fails because `test_rank_methods_series()` in `tests/test_stats.py` calls `Series.rank` with a non-keyword first argument, which is mistakenly assigned to `axis` (as this is the first parameter in the new signature, which is the one coming from `DataFrame.rank`) instead of `method` (which was the first parameter in the old signature of `Series.rank`).

So there is a change in the API. Can I alter the test? Is there a warning I should put somewhere?

@jreback The problem is that, to perform the aggregation, different methods are tried (when one fails, the exception is caught and the next is tried), and at a last resort the aggregation is tried "item by item" recursively. For each item, we thus start over, until we obtain something or there is no item left to work with in the final recursive attempt. If any exception is raised, the current item is discarded.

Currently, when there is no child item left to work with a `ValueError` is raised, which is caught as any other error by the previous level in the recursion. The parent item which caused it is thus discarded. In fact, in the test I wrote (`.test_rank()` in `pandas/tests/test_groupby.py`) all items are discarded and we end up with an empty frame, instead of getting a meaningful error.

Fundamentally, the issue is that we need to be able to tell the difference between a real `ValueError` and a `ValueError` which means "sorry no item left", so that we may prevent returning an empty frame and throw a good error.

I'm going to propose another implementation, which I hope will be clearer.

@nbonnotte yes, don't mix 2 issues within a PR like this. too confusing what you are trying to do.

@jreback  Ok, I will do another PR with only the part where I put `Series.rank` and `DataFrame.rank` into `generic.py`, so that we can move on from this.

Note that it will **not** solve issue #11759, but it will be the first step.

@nbonnotte prob the way to actually solve this is to define `.rank` as a groupby function which handles this in the correct manner.

@jreback before defining `.rank` as a groupby function as you suggested, I'm trying this last approach, which seems to work.

But to be honest, I'm not sure I understand the reason behind each piece of code I touched, and I'm trusting Travis to tell me if I did something wrong. So let me know if I've done something I shouldn't have.

But if it seems OK to you, I'll squash the commits, with a proper commit message.

(Uh, the numpy_dev tests fail, but I can't see if that's related to my modifications)

All green

ok, think we are ready for this. pls rebase/update

@jreback done (and it's green)

you haven't changed like I indicated. you can simply add a `.rank(...)` function to groupby.

@jreback uh, sorry. Like that?

I still need to include the docstring, though. I guess I can pick it up from somewhere?...

you need to define a `.rank(..)` method on groupby.

So that I can put a docstring just for `GroupBy.rank`? I see.

But I still need to have different `SeriesGroupBy.rank`, `DataFrameGroupBy.rank`, `PanelGroupBy.rank` method.

I've added a commit, let me know what you think of it.

@nbonnotte this needs to define a `.rank` function and remove all of the changes you have made. IOW the try: except: stuff.

I don't understand how to do that without creating inconsistencies.

Currently on master:

```
In [4]: df = pd.DataFrame({'a':['A1', 'A1', 'A1'], 'b':['B1','B1','B2'], 'c':1})

In [5]: df.set_index('a').groupby('c').rank(method='first')
Out[5]:
Empty DataFrame
Columns: []
Index: []

In [6]: df.set_index('a').groupby('c').mean()
---------------------------------------------------------------------------
DataError                                 Traceback (most recent call last)
<ipython-input-6-b948f8cac388> in <module>()
----> 1 df.set_index('a').groupby('c').mean()

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in mean(self)
    933         """
    934         try:
--> 935             return self._cython_agg_general('mean')
    936         except GroupByError:
    937             raise

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in _cython_agg_general(self, how, numeric_only)
   3046     def _cython_agg_general(self, how, numeric_only=True):
   3047         new_items, new_blocks = self._cython_agg_blocks(
-> 3048             how, numeric_only=numeric_only)
   3049         return self._wrap_agged_blocks(new_items, new_blocks)
   3050

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in _cython_agg_blocks(self, how, numeric_only)
   3092
   3093         if len(new_blocks) == 0:
-> 3094             raise DataError('No numeric types to aggregate')
   3095
   3096         return data.items, new_blocks

DataError: No numeric types to aggregate
```

To be consistent, we'd like to throw an error. But things get tricky when we add another numeric column. Currently:

```
In [12]: dg = pd.DataFrame({'a': ['A1', 'A1', 'A1'],
                         'b': ['B1', 'B1', 'B2'],
                         'c': 1, 'd':2})

In [13]: dg.set_index('a').groupby('c').mean()
Out[13]:
   d
c
1  2

In [14]: dg.set_index('a').groupby('c').rank(method='first')
Out[14]:
    d
a
A1  1
A1  2
A1  3
```

But, with a simple `GroupBy.rank` without any `try ... except ...` as you suggest, I don't see how to avoid:

```
In [22]: dg = pd.DataFrame({'a': ['A1', 'A1', 'A1'],
                         'b': ['B1', 'B1', 'B2'],
                         'c': 1, 'd':2})

In [23]: dg.set_index('a').groupby('c').mean()
Out[23]:
   d
c
1  2

In [24]: dg.set_index('a').groupby('c').rank(method='first')
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-8fd2be978f65> in <module>()
----> 1 dg.set_index('a').groupby('c').rank(method='first')

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in rank(self, axis, method, numeric_only, na_option, ascending, pct)
   1426                               ascending=ascending, pct=pct)
   1427
-> 1428         return self.apply(curried)
   1429
   1430

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in apply(self, func, *args, **kwargs)
    638         # ignore SettingWithCopy here in case the user mutates
    639         with option_context('mode.chained_assignment', None):
--> 640             return self._python_apply_general(f)
    641
    642     def _python_apply_general(self, f):

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in _python_apply_general(self, f)
    642     def _python_apply_general(self, f):
    643         keys, values, mutated = self.grouper.apply(f, self._selected_obj,
--> 644                                                    self.axis)
    645
    646         return self._wrap_applied_output(keys, values,

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in apply(self, f, data, axis)
   1534             # group might be modified
   1535             group_axes = _get_axes(group)
-> 1536             res = f(group)
   1537             if not _is_indexed_like(res, group_axes):
   1538                 mutated = True

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in f(g)
    634         @wraps(func)
    635         def f(g):
--> 636             return func(g, *args, **kwargs)
    637
    638         # ignore SettingWithCopy here in case the user mutates

/Users/bonnotte/Git/pandas/pandas/core/groupby.py in curried(x)
   1424             return klass.rank(x, axis=0, method=method,
   1425                               numeric_only=numeric_only, na_option=na_option,
-> 1426                               ascending=ascending, pct=pct)
   1427
   1428         return self.apply(curried)

/Users/bonnotte/Git/pandas/pandas/core/generic.py in rank(self, axis, method, numeric_only, na_option, ascending, pct)
   4065         if numeric_only is None:
   4066             try:
-> 4067                 return ranker(self)
   4068             except TypeError:
   4069                 numeric_only = True

/Users/bonnotte/Git/pandas/pandas/core/generic.py in ranker(data)
   4057             ranks = algos.rank(data.values, axis=axis, method=method,
   4058                                ascending=ascending, na_option=na_option,
-> 4059                                pct=pct)
   4060             ranks = self._constructor(ranks, **data._construct_axes_dict())
   4061             return ranks.__finalize__(self)

/Users/bonnotte/Git/pandas/pandas/core/algorithms.py in rank(values, axis, method, na_option, ascending, pct)
    397         f, values = _get_data_algo(values, _rank2d_functions)
    398         ranks = f(values, axis=axis, ties_method=method,
--> 399                   ascending=ascending, na_option=na_option, pct=pct)
    400
    401     return ranks

/Users/bonnotte/Git/pandas/pandas/algos.pyx in pandas.algos.rank_2d_generic (pandas/algos.c:18472)()
    693                         ranks[i, argsorted[i, z]] = j + 1
    694                 elif tiebreak == TIEBREAK_FIRST:
--> 695                     raise ValueError('first not supported for '
    696                                      'non-numeric data')
    697                 elif tiebreak == TIEBREAK_DENSE:

ValueError: first not supported for non-numeric data
```

What do you suggest I do, precisely?

as I said you need to add a `.rank(...)` method on groupby that handles the kw args. see how `.first()` works now

Now I'm lost. Are you talking about `NDFrame.first`?

https://github.com/pydata/pandas/blob/master/pandas/core/groupby.py#L1029

Ok... I don't really understand how all this work (yet), but I'll try.

So basically `.first()` is defined with `_groupby_function`. If I try to do the same thing for `rank`, since that is not a cython function (is it?) I'm gonna end up with a variation of `self.aggregate(lambda x: algos.rank(x, ...))`. Right?

Now, I have a difficulty: `aggregate` seems to expect that `rank` will yield an aggregated value. But it is not an aggregation function... Clearly I'm not going in the right direction.

Could you be a little more specific, please?

@nbonnotte just trace some other functions thru. `.rank` is a transformer.

The other functions are `sum`, `prod`, `min`, `max`, `last`, which all give aggregated values. What other functions are you alluding to? 

search for transform. eg. `cumprod`

Now I'm getting somewhere :)

But I still get an error when there is a mix of numeric and non-numeric data:

```
ValueError: transform must return a scalar value for each group
```

thats a new error message FYI.

this is not really a transform, its much more like `.cumcount`. I know I keep pointing at different things, but have to find the closest thing. That's what I mean, you simply need to define it as a function (that can take args). As opposed to being dispatched.

`.cumcount` is really peculiar, I don't see how I could reproduce its mechanism.

My last commit seems to be working, but it's a bit dirty. 

@jreback what do you think of this last version?

In any case, there are still a few other things I'd like to test. I'm not sure I'm backward compatible, and at least I'd like to know if, how, and why.

ok looking a lot better! a few comments

can you rebase

## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/11918?src=pr) is 85.32% (diff: 94.11%)

> Merging [#11918](https://codecov.io/gh/pydata/pandas/pull/11918?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will increase coverage by **<.01%**

``` diff
@@             master     #11918   diff @@
==========================================
  Files           141        141          
  Lines         50679      50696    +17   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43240      43256    +16   
- Misses         7439       7440     +1   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [474fd05...f432b2b](https://codecov.io/gh/pydata/pandas/compare/474fd0519b768efd1c7ec6555930873bf9910e00...f432b2b51e3fe881c51bfce817b026bf74d2e270?src=pr)

I've rebased, but I still need to work a bit on this.