BUG: Preserve subclassing with groupby operations
State: closed

Preserve subclassing with groupby operations

- [x] closes #28330
- [x] tests added / passed
- [x] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Groupby
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
Hello @alkasm! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





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

##### Comment last updated at 2019-09-29 08:12:48 UTC
@alkasm the fixes here look great.  can you rebase and address comments about making the tests more thorough
pls also merge master
@alkasm can you rebase
Sorry for the delay y'all. Should be able to clean up, write new tests, and rebase this week sometime.
Due to the many changes from #28835 I found it easier to just make a new branch. Here's my current branch off master: https://github.com/alkasm/pandas/tree/alkasm/groupby-preserve-subclass (I can force push the new changes to this current branch, or just open a new PR if it's more helpful).

However there's a few issues I'm running into getting this to work in all cases; and I'm having trouble locating the source of the issues.

Borrowing from `tests/groupby/test_transform.py` and the suggestion from @WillAyd to use `conftest.reduction_kernels`, I wrote a short script to test the transforms and aggregations to see if the subclasses are returned properly:

```python
import pandas as pd
import numpy as np
from pandas.util.testing import SubclassedSeries, SubclassedDataFrame
from pandas.tests.groupby.conftest import reduction_kernels

objs = [
    SubclassedDataFrame(
        dict(a=[0, 0, 0, 1, 1, 1], b=range(6)), index=["A", "B", "C", "D", "E", "F"]
    ),
    SubclassedSeries([0, 0, 0, 1, 1, 1], index=["A", "B", "C", "D", "E", "F"]),
]

for obj in objs:
    g = obj.groupby(np.repeat([0, 1], 3))
    for k in reduction_kernels:
        if k in ("ngroup", "size"):
            continue
        args = {"nth": [0], "quantile": [0.5]}.get(k, [])
        xform = g.transform(k, *args)
        agg = g.agg(k, *args)
        if not (type(obj) == type(xform) == type(agg)):
            print(f"{k:>10}, {obj.__class__.__name__}, {xform.__class__.__name__}, {agg.__class__.__name__}")
```

and from running this script, it's seen that three of the aggregation functions don't produce the proper subclasses (i.e. somewhere they are not using `self.obj._constructor`):

```
      skew, SubclassedDataFrame, SubclassedDataFrame, DataFrame
   nunique, SubclassedDataFrame, SubclassedDataFrame, DataFrame
       mad, SubclassedDataFrame, SubclassedDataFrame, DataFrame
```

I can't really find how these functions end up constructing their result as a `DataFrame`. Can anyone with some more knowledge here point me in the right direction? Hopefully that test is sufficient so that this is otherwise ready to go.
can you merge master
Based on a quick look, I think you're best bet is in core.groupby.generic `_wrap_applied_output` methods.

I also recently noticed that core.apply has a place where it does `f(Series([]))` instead of `f(obj._constructor([]))` that might not play well with subclassing.
can you merge master
@alkasm can you fix up merge conflicts?
can you merge master and will look again
@alkasm do you have time to fix the merge conflicts? We're doing a release this week.
Nice PR but looks like this has gone stale. Ping if you'd like to pick back up and can address merge conflicts / comments
I recently came across this bug/PR while working on a project that uses a subclassed `DataFrame`. It seems like this PR has been closed and the issue removed from "Contributions Welcome", but the initial issue #28330 is still open. 

I just wanted to check on the current status here -- it seems like this PR was abandoned. If this were to be picked up, would you recommend starting from here or in a fresh PR?
@JBGreisman if interested you can merge these changes into your own local branch and push up a new PR
@JBGreisman thank you for picking this back up!
@alkasm No worries -- thanks for laying the groundwork!