Skip to content

[ENH] Allow pad, backfill and cumcount in groupby.transform #34453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Jun 3, 2020
Merged

[ENH] Allow pad, backfill and cumcount in groupby.transform #34453

merged 7 commits into from
Jun 3, 2020

Conversation

fujiaxiang
Copy link
Member

@fujiaxiang fujiaxiang changed the title Allow pad, backfill and cumcount in groupby.transform [ENH] Allow pad, backfill and cumcount in groupby.transform May 29, 2020
@fujiaxiang
Copy link
Member Author

Worth noting that the added aliases pad -> ffill and backfill -> bfill in pandas/core/generic.py are not necessary for resolving issue #31269.

However, the same thing was done in pandas/core/groupby/groupby.py so I thought it'd be nice to add these 2 aliases too. Besides, it makes the test test_transform_transformation_func simpler because of the added consistency.

Do let me know if you guys think this is inappropriate. I will remove them.

WillAyd
WillAyd reviewed May 29, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think looks pretty good - quick question

@fujiaxiang fujiaxiang requested a review from WillAyd May 30, 2020 00:39
@fujiaxiang
Copy link
Member Author

@WillAyd btw should I mention the 2 new aliases in whatsnew? I don't have an issue number to tag it to though. Should I create an issue for that?

@jreback jreback added this to the 1.1 milestone May 31, 2020
@jreback jreback added the Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate label May 31, 2020
jreback
jreback requested changes May 31, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fujiaxiang this lgtm. I would add the aliases in the reference api section though (for series/frame); seems ffill, bfill are also not there. ping on green.

@fujiaxiang
Copy link
Member Author

ping

@fujiaxiang fujiaxiang requested a review from jreback June 2, 2020 15:16
jreback
jreback reviewed Jun 2, 2020
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. tiny comment on Series.replace and question about aliases

doc/source/reference/series.rst
@@ -214,11 +214,17 @@ Missing data handling
.. autosummary::
:toctree: api/

Series.isna
Series.notna
Series.backfill
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorisvandenbossche is there any way we can indicate aliases here? (should we just not list them)?

@WillAyd WillAyd merged commit ea402a3 into pandas-dev:master Jun 3, 2020
@WillAyd
Copy link
Member

WillAyd commented Jun 3, 2020

Thanks @fujiaxiang another great PR - keep at it!

@fujiaxiang fujiaxiang deleted the allow_pad_backfill_cumcount_in_groupby_transform branch June 4, 2020 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WillAyd WillAyd WillAyd approved these changes

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Groupby Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
Projects
None yet
Milestone
1.1
Development

Successfully merging this pull request may close these issues.

ENH: allow 'pad', 'backfill' and 'cumcount' in groupby.transform
3 participants
@fujiaxiang @WillAyd @jreback