CLN: fix asv setup_cache() for groupby.AggFunctions benchmarks
State: closed

This PR fixes a typo in an `asv` benchmark and additionally clarifies a misleading result.

Simply fixing the typo leads to this rather surprising result where the single column benchmark is ~6x slower than the multi-column benchmark:
```
$ asv dev -b groupby.AggFunctions
· Discovering benchmarks
· Running 4 total benchmarks (1 commits * 1 environments * 4 benchmarks)
[  0.00%] ·· Benchmarking existing-py_home_chris_anaconda3_bin_python
[ 12.50%] ··· Setting up groupby.py:213                                                                                                                                   ok
[ 12.50%] ··· groupby.AggFunctions.time_different_numpy_functions                                                                                                   53.7±0ms
[ 25.00%] ··· groupby.AggFunctions.time_different_python_functions_multicol                                                                                         84.7±0ms
[ 37.50%] ··· groupby.AggFunctions.time_different_python_functions_singlecol                                                                                         483±0ms
[ 50.00%] ··· groupby.AggFunctions.time_different_str_functions                                                                                                     46.0±0ms
```
Fortunately, the difference is solely due to the inclusion of a `str` column in the single column case. Therefore, we drop that column to keep the comparison comparable:
```
$ asv run HEAD -b groupby.AggFunctions
· Creating environments
· Discovering benchmarks..
·· Uninstalling from conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt...
·· Building d3fa9d67 <groupby_agg_functions> for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt.............................
·· Installing d3fa9d67 <groupby_agg_functions> into conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt....
· Running 53900 total benchmarks (13475 commits * 1 environments * 4 benchmarks)
[  0.00%] · For pandas commit d3fa9d67 <groupby_agg_functions>:
[  0.00%] ·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
[  0.00%] ··· Setting up groupby.py:213                                                                                                                                   ok
[  0.00%] ··· Running (groupby.AggFunctions.time_different_numpy_functions--)....
[  0.00%] ··· groupby.AggFunctions.time_different_numpy_functions                                                                                                   23.2±3ms
[  0.01%] ··· groupby.AggFunctions.time_different_python_functions_multicol                                                                                        70.2±10ms
[  0.01%] ··· groupby.AggFunctions.time_different_python_functions_singlecol                                                                                       38.5±10ms
[  0.01%] ··· groupby.AggFunctions.time_different_str_functions                                                                                                   22.4±0.3ms
```
- [ ] closes #xxxx
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Clean
name: Benchmark
description: Performance (ASV) benchmarks

Comments:
Hello @qwhelan! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`asv_bench/benchmarks/groupby.py`](https://github.com/qwhelan/pandas/blob/d3fa9d67670b022750ed9a1e248c52c25d3a3339/asv_bench/benchmarks/groupby.py) !


I'm not sure the benchmark is to blame here. I think this is more of a byproduct of agg giving inconsistent results, i.e.:

```python
df.groupby('key1').agg(sum)  
```

would yield something different than

```python
df.groupby('key1').agg([sum])
```

If you search there should be an open issue about this. Would rather fix that than change the benchmark, though `setup_cache` change itself would be OK
I believe this mostly been fixed in master
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=h1) Report
> Merging [#24097](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/8ea7744851048b032404f2400a7c7a070479e152?src=pr&el=desc) will **decrease** coverage by `49.68%`.
> The diff coverage is `n/a`.

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

```diff
@@             Coverage Diff             @@
##           master   #24097       +/-   ##
===========================================
- Coverage    92.2%   42.52%   -49.69%     
===========================================
  Files         162      161        -1     
  Lines       51729    51697       -32     
===========================================
- Hits        47697    21982    -25715     
- Misses       4032    29715    +25683
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `?` | |
| #single | `42.52% <ø> (-0.51%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/formats/latex.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvbGF0ZXgucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/core/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY2F0ZWdvcmljYWwucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/sas/sas\_constants.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfY29uc3RhbnRzLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/plotting.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvcGxvdHRpbmcucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/formats/html.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvaHRtbC5weQ==) | `0% <0%> (-98.62%)` | :arrow_down: |
| [pandas/core/groupby/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9jYXRlZ29yaWNhbC5weQ==) | `0% <0%> (-95.46%)` | :arrow_down: |
| [pandas/io/sas/sas7bdat.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXM3YmRhdC5weQ==) | `0% <0%> (-91.17%)` | :arrow_down: |
| [pandas/io/sas/sas\_xport.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfeHBvcnQucHk=) | `0% <0%> (-90.15%)` | :arrow_down: |
| [pandas/core/sparse/scipy\_sparse.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL3NjaXB5X3NwYXJzZS5weQ==) | `10.29% <0%> (-89.71%)` | :arrow_down: |
| ... and [119 more](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24097?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/24097?src=pr&el=footer). Last update [8ea7744...26d1e65](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=h1) Report
> Merging [#24097](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/03134cb92ed61d073438daac7a7c2a0e094efede?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #24097      +/-   ##
==========================================
+ Coverage    92.2%    92.2%   +<.01%     
==========================================
  Files         162      162              
  Lines       51701    51701              
==========================================
+ Hits        47671    47672       +1     
+ Misses       4030     4029       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.6% <ø> (ø)` | :arrow_up: |
| #single | `43.03% <ø> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `87.41% <0%> (-0.1%)` | :arrow_down: |
| [pandas/io/json/json.py](https://codecov.io/gh/pandas-dev/pandas/pull/24097/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vanNvbi5weQ==) | `93.09% <0%> (+0.47%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24097?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/24097?src=pr&el=footer). Last update [03134cb...91cd3e7](https://codecov.io/gh/pandas-dev/pandas/pull/24097?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@WillAyd I don't believe that case is being hit here - the issue is that the multicolumn case aggregates for columns `value1`, `value2`, `value3`, which are all numeric (and thus fast due to cython path). The single column case also includes the column `key2`, which is a `str` column and thus must use the slower pure python path.

These factors lead to the misleading output of the single col case being 6x slower than the multicol case.

@jreback The typo was fixed in master, but there's an issue in the benchmark definition still.
My point is that the aggregations should not be applied to the object column. It only seems to do that when supplying an iterable of functions, whereas applying individually does the right thing
@WillAyd Okay, I can add separate benchmarks for the single function case but the intent of the existing benchmark seems to be to compare multiple aggregation performance on numeric data.
@qwhelan is this still relevant?