BUG: Preserve subclassing with groupby operations

[alkasm]

Preserve subclassing with groupby operations

• closes groupby and resample methods do not preserve subclassed data structures #28330
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[TomAugspurger]

Thanks for the PR. Looks good overall.

[pep8speaks]

Hello @alkasm! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

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

Comment last updated at 2019-09-29 08:12:48 UTC

[jreback]

ideally we split these into several tests, but could be a followup

[jreback]

test change, pls merge master, ping on green.

[jreback]

actually these tests are different that the above ones, can you make a new test

[jreback]

also pls parameterize over as many functions that you can here

[WillAyd]

You can use reduction_func from conftest. Probably worth adding something similar for transforming functions as well

[jbrockmendel]

@alkasm the fixes here look great. can you rebase and address comments about making the tests more thorough

[jreback]

can you update to comments here

[jreback]

pls also merge master

[jbrockmendel]

@alkasm can you rebase

[alkasm]

Sorry for the delay y'all. Should be able to clean up, write new tests, and rebase this week sometime.

[alkasm]

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:

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.

[jreback]

can you merge master

[jbrockmendel]

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.

[jreback]

can you merge master

[WillAyd]

@alkasm can you fix up merge conflicts?

[jreback]

can you merge master and will look again

[TomAugspurger]

@alkasm do you have time to fix the merge conflicts? We're doing a release this week.

[WillAyd]

Nice PR but looks like this has gone stale. Ping if you'd like to pick back up and can address merge conflicts / comments

[JBGreisman]

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?

[WillAyd]

@JBGreisman if interested you can merge these changes into your own local branch and push up a new PR

[alkasm]

@JBGreisman thank you for picking this back up!

[JBGreisman]

@alkasm No worries -- thanks for laying the groundwork!