ENH: better dtype inference when doing DataFrame reductions
State: closed

- [x] closes #40669, #42895 & #14162

Supercedes #52707 and possibly #52261, depending on reception.

This PR improves on #52707 by not attempting to infer the combined dtypes from the original dtypes and builds heavily on the ideas in #52261, but avoids the overloading of `kwargs` in the extensionarray functions/methods in that PR. Instead this  keeps all  the work in `ExtensionArray._reduce` by adding an explicit `keepdims` param to that method signature. This makes this implementation simpler than #52261 IMO.

This PR is not finished, and would I appreciate feedback about the direction here compared to #52261 before I proceed. This  works currently correctly for masked arrays & ArrowArrays (AFAIKS), but still need the other extensionArray subclasses (datetimelike arrays & Categorical). I've written some tests, but need to write more + write the docs.

CC: @jbrockmendel & @rhshadrach 

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Reduction Operations
description: sum, mean, min, max, etc.

Comments:
The reason for the gymnastics/checking for keepdims in #52261 is bc 3rd party EAs won't have implemented keepdims, so will need a deprecation cycle to catch up (though in general I'd be OK with going less far out of our way with this kind of thing)
I've refactored to use `_reduce_with_wrap` method instead of a keyword. This avoids the deprercation issue raised by @jbrockmendel.
> I've refactored to use _reduce_with_wrap method instead of a keyword. This avoids the deprercation issue

This still requires downstream authors to implement something new, just avoids issuing a warning to inform them about it.


Actually it is backward compatible: If they don't implement this, their scalar just gets wrapped in a numpy array, so will work unchanged. So it's backward compatible, they just don't get this improved type inference without implementing `_reduce_with_wrap` in their arrays.

I've updated, so it now works correctly with pyarrow and categorical dtypes. Datetimelikes still missing, likewise docs and maybe some additional tests. So now we can do (notice the dtype):

```python
>>> arr = pd.array([1, 2], dtype="int64[pyarrow]")
>>> df = pd.DataFrame({"a": arr, "b": arr})
>>> df.median()
a    1.5
b    1.5
dtype: double[pyarrow]
```

This PR has been rebased to be based on #52890. Could be an idea to get that merged 
After the various perf PRs ( #52998, #53013 & #53040), I now get unchanged perf for axis=0 when calling `asv continuous -f 1.1 upstream/main HEAD -b stat_ops.FrameOps`:

```
       before           after         ratio
     [4f14b456]       [5760ee38]
     <master>         <reduction_dtypes_II>
+      3.90±0.02s       12.7±0.01s     3.25  stat_ops.FrameOps.time_op('median', 'Int64', 1)
+      4.22±0.01s       6.68±0.01s     1.58  stat_ops.FrameOps.time_op('kurt', 'Int64', 1)
+         3.96±0s       5.86±0.01s     1.48  stat_ops.FrameOps.time_op('skew', 'Int64', 1)
+      2.94±0.01s       3.82±0.01s     1.30  stat_ops.FrameOps.time_op('mean', 'Int64', 1)
+         2.34±0s       3.03±0.01s     1.29  stat_ops.FrameOps.time_op('sum', 'Int64', 1)
+      2.35±0.01s       3.01±0.01s     1.28  stat_ops.FrameOps.time_op('prod', 'Int64', 1)
+      3.70±0.01s       4.58±0.01s     1.24  stat_ops.FrameOps.time_op('std', 'Int64', 1)
+      3.63±0.01s       4.50±0.01s     1.24  stat_ops.FrameOps.time_op('var', 'Int64', 1)
-     3.46±0.03ms      3.08±0.02ms     0.89  stat_ops.FrameOps.time_op('median', 'Int64', 0)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```

That means that `axis=0` is good now, while for `axis=1` is slower because it's in effect the same as doing `df.T.sum(axis=0)` on a (4, 100_000,)-shaped DataFrame, where the transpose is very slow (but can be implroved by #52836 or #52083) + there is a lot of work to extract masked  data from 100_000 ExtensionBlocks. So I think this is as good as it gets for `axis=1`, unless we can avoid transposing the dataframe, which will speed things up tremendously, but that's unrelated to this PR.
I've improved the `median` with axis=1 case, so it now falls in line with the other axis=1 reductions

```
       before           after         ratio
     [4f14b456]       [122488b1]
     <master>         <reduction_dtypes_II>
+      4.25±0.01s       6.68±0.02s     1.57  stat_ops.FrameOps.time_op('kurt', 'Int64', 1)
+      4.64±0.05s          6.42±0s     1.38  stat_ops.FrameOps.time_op('sem', 'Int64', 1)
+         2.35±0s       3.04±0.01s     1.29  stat_ops.FrameOps.time_op('sum', 'Int64', 1)
+      2.96±0.01s          3.81±0s     1.29  stat_ops.FrameOps.time_op('mean', 'Int64', 1)
+      3.60±0.04s          4.53±0s     1.26  stat_ops.FrameOps.time_op('var', 'Int64', 1)
+      3.70±0.01s          4.65±0s     1.26  stat_ops.FrameOps.time_op('std', 'Int64', 1)
+      3.92±0.01s       4.85±0.02s     1.24  stat_ops.FrameOps.time_op('median', 'Int64', 1)
+     1.36±0.05ms      1.50±0.07ms     1.11  stat_ops.FrameOps.time_op('kurt', 'Int64', 0)
-     3.45±0.07ms      3.02±0.04ms     0.88  stat_ops.FrameOps.time_op('median', 'Int64', 0)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```

Generally, in the `axis=1` cases in the tests, we now have to concatenating the reduction result for the 100_000 masked arrays.  Before we only had to concatenate 100_000 numpy arrays, which was inherently faster. OTOH doing concatenation over 100_000 masked arrays is always slow, so can't become much faster without a significant rewrite which will be out of scope here.
 
id expect this to be very close to perf-neutral.  where is the slowdown coming from?
> id expect this to be very close to perf-neutral. where is the slowdown coming from?

It is performance neutral when the dataframe is not-wide, but when the dataframe gets very wide, there will be a certain slowdown, because pandas will have to create 1 new masked array per column and then join those masked arrays. Joining masked arrays is slower than joining the same number of ndarrays, because the masked array have two ndarrays internally + the masked arrays are written in python while the ndarrays are written in C, which gives some overhead for masked arrays.

Having said that, note that the ASV slowdowns that I show is about  joining 100.000 columns, i.e quite a lot. I think the performance is actually ok, taking the above explanation into account. 
Ping... The slowdown is for very wide arrays, so I don't think this is a practical issue, and the dtype inference when doing reduction is a very nice UX to get into pandas IMO.
Do we know/suspect the `keepdims` version of this would experience the same slowdown?
> Do we know/suspect the `keepdims` version of this would experience the same slowdown?

This PR does use `keepdims=True`, so I don't understand the question. Do you mean #52261?

Generally about performance here, the slower test is about aggregating 100.000 columns each being of length 2 and rejoining the result values into a series. In this scenario each aggregation will always be lightning fast, because each array is of length just 2. If the size of the data was different, the aggregation calculation cost per column would be much higher and that would dwarf the slowdown we see here. So the issue is specific to wide and short dataframes (which are rare) +  when aggregating over axis=1 (much rarer than aggregation over axis=0, but is already slow from the need to transform) and in return we get better dtype handling for masked arrays (which is very useful, so we solve a UX painpoint from using masked arrays by doing this IMO).

> This PR does use `keepdims=True`, so I don't understand the question. Do you mean #52261?

Yes.

> Generally about performance here...

I think as-is this change is worth the performance impact we're seeing. I also agree with your assessment here except...

> So the issue is specific to wide and short dataframes (which are rare)

One gets a wide, short DataFrame when using `axis=1` on a long narrow DataFrame with certain ops. In my personal experience, both wide and short DataFrames as well as using `axis=1` are quite rare, but I don't know if that is also the experience of users. 
I'd like to give this another once-over, but am officially declaring that im OK with the perf impact
> > So the issue is specific to wide and short dataframes (which are rare)
> 
> One gets a wide, short DataFrame when using `axis=1` on a long narrow DataFrame with certain ops. In my personal experience, both wide and short DataFrames as well as using `axis=1` are quite rare, but I don't know if that is also the experience of users.

Yeah, I agree. I made an attempt to address that (`when aggregating over axis=1 (much rarer than aggregation over axis=0, but is already slow from the need to transform`), though I meant transpose, not transform.
The top post mentions "https://github.com/pandas-dev/pandas/issues/51446 (the last one intended, not done yet)". Has that changed in the meantime? (just to ensure you update the top post to close/not close that issue)
This fails currently, but will pass again after #53482 is merged.
I think I've addressed all comments, but a decision is needed about doing this as a separate method, or as a key word in `_reduce`. That discussion is [here](https://github.com/pandas-dev/pandas/pull/52788#discussion_r1209527421).
Ping. I think I need a decision if the `_reduce_and_wrap` way is the way forward or not.
I haven't re-reviewed in a while, but I was happy here as of the last time I looked at it.  If Joris is on board, let's do it.
I've actually just this morning made a new version using `_reduce` only (i.e. scrapping `_reduce_and_wrap`)`. I prefer this new version (because we now only have one reduction methods instead of two), but if the other is preferred, it is easy to revert back.

I maintain backward compat in this new version by:
1. making the `keepdim` parameter keyword only
2. only calling it with `keepdims=True` if the `_reduce` method signature has a parameter named "keepdims"
3. if the `_reduce` method signature does not have a parameter named "keepdims", `_reduce` gets called without supplying the `keepdims` parameter, we emit a `FutureWarning` and take care of wrapping the (scalar) reduction result in a ndarray before passing it on.

This is possible because `_reduce_and_wrap` was actually only called inside the `blk_func` inside `DataFrame._reduce`, so by doing some introspection there we can keep backward compat. See new version:

https://github.com/pandas-dev/pandas/blob/f85deab32d337775f6552d092f6bf3054fb60b88/pandas/core/frame.py#L10874-L10895.

Notice especially the FutureWarning starting on line 10885. This will allow us to not require `keepdims` now, even though `keepdims` is in the signature of `ExtensionArray._reduce`.  In v3.0, we will drop the signature checking and only call `values._reduce` with `keepdims=True`, i.e. it will fail without a `keepdims` parameter in v3.0.

Check out the `test_reduction_without_keepdims` test in `pandas/tests/extension/decimal/test_decimal.py` for a test of what happens when extensionarrays don't have a keepdim parameter in their `_reduce` method.

Thoughts? I prefer this new version, but it's is easy to revert back if needed.
Ping...
I've updated, added  some  + fixed some dtype issues with `DataFrame.any/all`.
Thanks this is an awesome improvement @topper-123 
nice! thanks for sticking with this @topper-123 
Yes, but worth the slow process, I wouldn't have thought of this way to add a parameter to `_reduce` in a backward compatible way if this had gone faster 😄.