ENH: Add numba engine to groupby.aggregate

[mroeschke]

• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

In the same spirit of #31845 and #32854, adding engine and engine_kwargs arguments to groupby.aggreate.

This PR has some functionality that is waiting to be merged in #32854

[jbrockmendel]

is it worth making a _numba_agg to call instead of adding a clause to python_agg_general? this code is difficult to debug so to the extent things can be isolated thatd be nice

[mroeschke]

@jbrockmendel sure I can create a separate _numba_agg function. I just noticed that python_agg_general contains the essentially what I want _numba_agg to do, but definitely nicer if I don't have to traverse all these calls to get to python_agg_general

[mroeschke]

So by creating a new _numba_agg function, I would be repeating a lot of the iterating over groups + result boxing functions present in python_agg_general and elsewhere. Is that okay?

[pep8speaks]

Hello @mroeschke! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-04-24 05:09:27 UTC

[jbrockmendel]

So by creating a new _numba_agg function, I would be repeating a lot of the iterating over groups + result boxing functions present in python_agg_general and elsewhere. Is that okay?

I trust your judgement.

[jbrockmendel]

E TypeError: _percentile_dispatcher() missing 1 required positional argument: 'q'

[jbrockmendel]

LGTM

[jreback]

looks good, just a couple of comments.

also for followups

• updating a section in the groupby docs with an example on how to use numba (for rolling as well if we don't have it).
• update doc-string? also IIRC this raises on failures rather than trying to fall back like non-numba .agg, let's note that in doc-string & in the groupby docs (as a warning i think).
• cleanups as noted

[jreback]

can you condense these to a single function call (with whatever return args you need later on)
you can certainly leave these functions individually in core.util.numba_, just when you are calling it would make th api simpler here. (also if you can do this simplification other places we call numba).

can do this in a followup.

[mroeschke]

Sure can follow up with this cleanup

[jreback]

can you actually just check the class type here?

[mroeschke]

I raise ValueError from the numba utilities, so I need to distinguish between ValueErrors from another op vs the numba utilties.

Should I make a NumbaUtilError(ValueError) exception? Then we would also need to expose it to users

[jreback]

yes, let's have a separate exception (but in a followon)

[mroeschke]

Here is some example timeit benchmarks

   In [1]: N = 10 ** 3

   In [2]: data = {0: [str(i) for i in range(100)] * N, 1: list(range(100)) * N}

   In [3]: df = pd.DataFrame(data, columns=[0, 1])

   In [4]: def f_numba(values, index):
      ...:     total = 0
      ...:     for i, value in enumerate(values):
      ...:         if i % 2:
      ...:             total += value + 5
      ...:         else:
      ...:             total += value * 2
      ...:     return total
      ...:

   In [5]: def f_cython(values):
      ...:     total = 0
      ...:     for i, value in enumerate(values):
      ...:         if i % 2:
      ...:             total += value + 5
      ...:         else:
      ...:             total += value * 2
      ...:     return total
      ...:

   In [6]: groupby = df.groupby(0)
   # Run the first time, compilation time will affect performance
   In [7]: %timeit -r 1 -n 1 groupby.aggregate(f_numba, engine='numba')  # noqa: E225
   2.14 s ± 0 ns per loop (mean ± std. dev. of 1 run, 1 loop each)
   # Function is cached and performance will improve
   In [8]: %timeit groupby.aggregate(f_numba, engine='numba')
   4.93 ms ± 32.3 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

   In [9]: %timeit groupby.aggregate(f_cython, engine='cython')
   18.6 ms ± 84.8 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

[mroeschke]

Looks like the the aggregate and transform docstrings are not rendering correctly:

https://pandas.pydata.org/docs/reference/api/pandas.core.groupby.GroupBy.aggregate.html#pandas.core.groupby.GroupBy.aggregate
https://pandas.pydata.org/docs/reference/api/pandas.core.groupby.GroupBy.transform.html#pandas.core.groupby.GroupBy.transform

I can try to address those in a followup (in addition to adding to the agg docstring). But address updating groupby.rst in this PR

[jreback]

yes, let's have a separate exception (but in a followon)

[jreback]

thanks @mroeschke very nice. a followon request + plus any refactorings as discussed as follows would be great.