ENH Consistent apply output when grouping with freq

[stephen-hoover]

The BinGrouper.apply (used by the TimeGrouper) and BaseGrouper.apply (used by the Grouper) have different output types. To make them consistent, remove BinGrouper.apply and let it use the same method as the superclass BaseGrouper. This requires changing BinGrouper.groupings to return an object which looks sufficiently like a Grouping object for BaseGrouper.apply to be happy. Use a namedtuple to create an object with the needed attributes.

I think this change will only affect the output of apply with a custom function, and only when grouping by a TimeGrouper (although a Grouper with a freq specified turns into a TimeGrouper, so that counts).

Closes #11742 .

[jreback]

tests!

[stephen-hoover]

I wanted to make sure it wouldn't be outright rejected first. :-) I'll write tests now.

[stephen-hoover]

Tests. On the master branch, TestGroupBy.test_timegrouper_apply_return_type_series fails, and TestGroupBy.test_timegrouper_apply_return_type_value raises a TypeError.

[jreback]

create an actual Grouping object here rather than a mock

[stephen-hoover]

You're right, that looks a lot better. Fixed.

[jreback]

Can u have a look thru online groupby issues (there are a few!) and see if this has some similarity with any

I recall one that had to do with the dtype changing in apply (and had to do with that code u removed) so maybe this would fix another issue

[stephen-hoover]

A search through issues tagged "Groupby", or containing "TimeGrouper" and "apply", or "Grouper", "apply", and "freq" doesn't show anything that looks to me like it would be fixed by this.

[jreback]

your PR might close #5839

[jreback]

i'll have to look at this more, but pls add a whatsnew

[stephen-hoover]

I added a What's New entry. Not sure the best way to describe / categorize this -- perhaps it belongs under bugs?

It doesn't look like #5839 is affected. I checked to be sure, and the examples there are unchanged by this PR.

This PR only affects apply operations after a single groupby involving grouping times by a freq argument. And even then, something like df.groupby([pd.Grouper(freq='M', key='date')]).apply(lambda x: x.value.sum()) is unchanged. The only thing that changes is the output if you don't put the Grouper in a list (which is now consistent with the same thing using the Grouper in a list).

I scanned through the docs and didn't see any examples or instructions that would be affected by this PR.

[jreback]

can you also tests that these produce the same output when doing a .resample(..).apply(..) (both functions)

[jreback]

call this result instead of grp_out

[stephen-hoover]

I changed the name of grp_out to expected, since that's the thing left unchanged by this PR.

[jreback]

I am not convinced this is doing the right thing. notice the return types dtypes.

In [25]: df.groupby(pd.Grouper(key='date')).apply(sumfunc_series)
Out[25]: 
            sum
date           
10/10/2000   10
11/10/2000   13

In [26]: df.groupby(pd.Grouper(key='date')).apply(sumfunc_series).dtypes
Out[26]: 
sum    int64
dtype: object

[stephen-hoover]

I'm not sure if it's doing the right thing, but this PR doesn't change the output of df.groupby(pd.Grouper(key='date')).apply(sumfunc_series), and now that output type is consistent with the output of df_dt.groupby(pd.TimeGrouper(freq='M', key='date')).apply(sumfunc_series).

When you suggest testing the output of .resample(..).apply(..), you mean to compare the result to df_dt.set_index('date').resample('M').apply(sumfunc_series)? I would have expected that to be the same, but it's not. Also, this PR doesn't affect the output of .resample(..).apply(..). I don't know enough about the resample code path to say if it should. Outputs:

In [45]: df_dt.set_index('date').resample('M').apply(sumfunc_series)
Out[45]: 
            value
date             
2000-10-31    NaN
2000-11-30    NaN

In [46]: df_dt.set_index('date').resample('M').apply(sumfunc_value)
Out[46]: 
            value
date             
2000-10-31     10
2000-11-30     13

Corresponding outputs using a resampling groupby, this PR:

In [50]: df_dt.groupby(pd.Grouper(key='date', freq='M')).apply(sumfunc_series)
Out[50]: 
            sum
date           
2000-10-31   10
2000-11-30   13

In [51]: df_dt.groupby(pd.Grouper(key='date', freq='M')).apply(sumfunc_value)
Out[51]: 
date
2000-10-31    10
2000-11-30    13
Freq: M, dtype: int64

Outputs on master branch:

In [16]: df_dt.set_index('date').resample('M').apply(sumfunc_series)
Out[16]: 
            value
date             
2000-10-31    NaN
2000-11-30    NaN

In [17]: df_dt.set_index('date').resample('M').apply(sumfunc_value)
Out[17]: 
            value
date             
2000-10-31     10
2000-11-30     13

In [18]: df_dt.groupby(pd.Grouper(key='date', freq='M')).apply(sumfunc_series)
Out[18]: 
date           
2000-10-31  sum    10
2000-11-30  sum    13
dtype: int64

In [19]: df_dt.groupby(pd.Grouper(key='date', freq='M')).apply(sumfunc_value)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-19-4ef8c22ce5dd> in <module>()
----> 1 df_dt.groupby(pd.Grouper(key='date', freq='M')).apply(sumfunc_value)
...
TypeError: cannot concatenate a non-NDFrame object

The test failure is in pandas.tests.test_generic.TestDataFrame.test_to_xarray, which doesn't look related to my change in test_groupby.py.

[jreback]

you can rebase on master; the to_xarray issue is fixed

[stephen-hoover]

Rebased.

[jreback]

can you rebase/update (and move note to 0.18.1). and i'll take a look.

[stephen-hoover]

@jreback , done.

[jreback]

don't list the PR number, only the issue number. give some more context, like: .groupby(...).apply(...). I think a brief example might be waranted here (e.g. show old and new)

[stephen-hoover]

@jreback , I added extra detail and examples to the What's New, and rebased. Sorry for the delay.

[jreback]

make the new an ipython block (so the code runs)

[jreback]

thanks @stephen-hoover

some doc-comments. ping when pushed and i can look.

[stephen-hoover]

@jreback , modified the docs. Let me know if this looks good.

[jreback]

thanks @stephen-hoover