Cythonized GroupBy Quantile
State: closed

I started this change with the intention of fully Cythonizing the GroupBy `describe` method, but along the way realized it was worth implementing a Cythonized GroupBy `quantile` function first. In theory we could concat together `count`, `mean`, `std`, `min`, `median`, `max`, and two `quantile` calls (one for 25% and the other for 75%) to get `describe`.

I decided to push this up as is because this alone is a decent amount of work and I think is worth getting reviewed before going further.

- [ ] closes #xxxx
- [X] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Relevant ASVs provided below:

```bash
       before           after         ratio
     [4a43815d]       [d7aec3fa]
-         236±6ms         421±40μs     0.00  groupby.GroupByMethods.time_dtype_as_field('float', 'quantile', 'transformation')
-         239±4ms         380±20μs     0.00  groupby.GroupByMethods.time_dtype_as_field('float', 'quantile', 'direct')
-         237±1ms          352±9μs     0.00  groupby.GroupByMethods.time_dtype_as_field('int', 'quantile', 'direct')
-         240±2ms         349±10μs     0.00  groupby.GroupByMethods.time_dtype_as_field('int', 'quantile', 'transformation')
-         348±6ms          372±9μs     0.00  groupby.GroupByMethods.time_dtype_as_group('int', 'quantile', 'transformation')
-         270±4ms         276±20μs     0.00  groupby.GroupByMethods.time_dtype_as_field('datetime', 'quantile', 'transformation')
-         357±6ms        347±0.9μs     0.00  groupby.GroupByMethods.time_dtype_as_group('int', 'quantile', 'direct')
-         271±4ms        263±0.6μs     0.00  groupby.GroupByMethods.time_dtype_as_field('datetime', 'quantile', 'direct')
-         515±9ms          351±2μs     0.00  groupby.GroupByMethods.time_dtype_as_group('datetime', 'quantile', 'transformation')
-         522±6ms          350±5μs     0.00  groupby.GroupByMethods.time_dtype_as_group('datetime', 'quantile', 'direct')
-         539±6ms          354±6μs     0.00  groupby.GroupByMethods.time_dtype_as_group('float', 'quantile', 'direct')
-         548±1ms          354±7μs     0.00  groupby.GroupByMethods.time_dtype_as_group('float', 'quantile', 'transformation')
```


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

Comments:
@WillAyd status?
Will rebase against master in the next few days and see what this looks like again
Hello @WillAyd! Thanks for updating the PR.









Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on February 28, 2019 at 07:01 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=h1) Report
> Merging [#20405](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/c9863865c217867583e8f6592ba88d9200601992?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/20405/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #20405      +/-   ##
==========================================
+ Coverage   91.74%   91.75%   +<.01%     
==========================================
  Files         173      173              
  Lines       52923    52941      +18     
==========================================
+ Hits        48555    48574      +19     
+ Misses       4368     4367       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.31% <100%> (ø)` | :arrow_up: |
| #single | `41.72% <7.69%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/groupby/groupby.py](https://codecov.io/gh/pandas-dev/pandas/pull/20405/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9ncm91cGJ5LnB5) | `97.18% <100%> (+0.38%)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/20405/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `87.57% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=footer). Last update [c986386...ec013bf](https://codecov.io/gh/pandas-dev/pandas/pull/20405?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@WillAyd LMK if I can be helpful in pushing this or #20024 over the finish line.
Thanks @jbrockmendel ! Haven't looked in detail yet but was planning to this weekend. I saw the failure was for Python 3.5 so off the top of my head I assume there's some kind of dict-ordering issue that may be at play.

Regardless if you take a look and get to it first then by all means go for it!
Hmm actually @jbrockmendel any chance you can reproduce the errors showing up on Travis? I ran the test suite of Python 3.5 macOS locally without issue so not sure why they are failing on CI
I can't reproduce this on OSX or Linux.  Maybe rebase?
@WillAyd Maybe rebase and hope it magically fixes itself?
At a loss for windows failures here - can anyone help point out where the int/float mismatch is popping up in the test case? Maybe @chris-b1 ?
Yep, I think that's the right way to do it. Any of the sized dtypes (int32,
int64, etc) won't be different on Windows, just the generic int.

On Mon, Nov 26, 2018, 6:32 PM William Ayd <notifications@github.com wrote:

> *@WillAyd* commented on this pull request.
> ------------------------------
>
> In pandas/core/groupby/groupby.py
> <https://github.com/pandas-dev/pandas/pull/20405#discussion_r236481255>:
>
> > +                raise TypeError("'quantile' cannot be performed against "
> +                                "'object' dtypes!")
> +            elif is_integer_dtype(vals):
> +                inferences['is_int'] = True
> +            elif is_datetime64_dtype(vals):
> +                vals = vals.astype(np.float)
> +                inferences['is_dt'] = True
> +
> +            return vals
> +
> +        def post_processor(vals):
> +            if inferences['is_dt']:
> +                vals = vals.astype('datetime64[ns]')
> +            elif inferences['is_int'] and interpolation in [
> +                    'lower', 'higher', 'nearest']:
> +                vals = vals.astype(np.int64)
>
> @chris-b1 <https://github.com/chris-b1> you were spot on with the issue -
> thanks! Is this the idiomatic way of casting back? Saw it elsewhere in the
> code base just not sure what implications may carry for windows
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/20405#pullrequestreview-178565479>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB1b_CN3gheOritQNrepwKNOCVcIm9_Jks5uzIg5gaJpZM4Svj6->
> .
>

Regarding the API change on a previous thread, I think the existing test case was actually covering up the issue, which is that the result of an `apply` operation seems to randomly add the `q` parameter as an index name. The test hid this inconsistency before by loosening the check against `agg` operations, but my last commit reverses that so it's clearer which test is likely failing.

This is probably something else to add to #13056
is there an issue about this ? (don't create one if not)
Hmm sorry got my issue numbers mixed up since I started this. There isn't one directly for this - it's progress towards https://github.com/pandas-dev/pandas/issues/19165 since this in theory could enable full Cythonization of `describe` (albeit in a separate PR) which would be the last thing on the list in that issue that I think we want to tackle
lgtm. pls merge master and ping on green.
Hmm I think green for the actual change. I think the failures are for any PRs with changes to the benchmarks so unrelated?
thanks @WillAyd