`SubClassedDataFrame.groupby().mean()` etc. use method of `SubClassedDataFrame`
State: closed

- [x] closes #51757
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [x] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

I appreciate that changing the behaviour for all subclasses is overkill, so pointers on how to better do this would be great

Labels:
name: Groupby
name: Subclassing
description: Subclassing pandas objects

Comments:
> I appreciate that changing the behaviour for all subclasses is overkill

I think that this might indeed be an issue for subclasses that don't override those methods. If we want to keep a fix like this, maybe checking whether the method is the one from pandas or not might be better (something like `self.obj.mean is Series.mean` instead of `type(self.obj) is Series`).

Because with the current change, if you have a subclass that doesn't influence how aggregations are done, you would get a big slowdown because of the current PR I think?
Is the WeightedDataFrame thing referenced in #51757 the motivating case here?

What if instead of this we did did something like

```
class DataFrame:
    @property
    def _gb_cls(self):
        return DataFrameGroupBy
```

and then had DataFrame.groupby return an instance of `_gb_cls`.  Then subclasses could override that.
> Is the WeightedDataFrame thing referenced in #51757 the motivating case here?
> 

Yes, our classes add a set of "weights" to the index which are used to calculate statistics such as the mean.

> What if instead of this we did did something like
> 
> ```
> class DataFrame:
>     @property
>     def _gb_cls(self):
>         return DataFrameGroupBy
> ```
> 
> and then had DataFrame.groupby return an instance of `_gb_cls`. Then subclasses could override that.

That would function, though feel this would be unexpected extra work for developers, so should be mentioned in the extending pandas section of the docs.

I was very surprised that groupby() doesn't just use the methods of its underlying class. If `DataFrame.mean()` and `Series.mean()` are themselves optimised, what is gained by `GroupBy` reimplementing them?

> If DataFrame.mean() and Series.mean() are themselves optimised, what is gained by GroupBy reimplementing them?

Performance.  The paths that iterate over groups generally make a sorted copy of the DataFrame to make that iteration faster, but the iteration itself is still costly.
> > If DataFrame.mean() and Series.mean() are themselves optimised, what is gained by GroupBy reimplementing them?
> 
> Performance. The paths that iterate over groups generally make a sorted copy of the DataFrame to make that iteration faster, but the iteration itself is still costly.

but, if these behaviours only change for subclasses, then performance is only lost for subclasses which override mean etc, in which case surely it is preferable that the intended results are returned rather than a fast one?
> but, if these behaviours only change for subclasses, then performance is only lost for subclasses which override mean etc, in which case surely it is preferable that the intended results are returned rather than a fast one?

The question was why we use a cython path instead of iterating over DataFrame.mean.  I gave an explanation of the status quo, not an argument against the fix suggested here.
> > but, if these behaviours only change for subclasses, then performance is only lost for subclasses which override mean etc, in which case surely it is preferable that the intended results are returned rather than a fast one?
> 
> The question was why we use a cython path instead of iterating over DataFrame.mean. I gave an explanation of the status quo, not an argument against the fix suggested here.

Apologies, I misunderstood the difference you were highlighting.

How do you think I should proceed?


> How do you think I should proceed?

Unless there is a viable way to handle your use case with an ExtensionArray (i.e. the weights would be somehow included with the values, not at the DataFrame level), then the approach here seems fine (albeit ugly)
Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.