-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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.
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
DOC: RT03 fix for various DataFrameGroupBy and SeriesGroupBy methods #57862
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @quangngd.
Very nice job, there is just one description that is not very clear, at least to me, see the comment.
pandas/core/groupby/generic.py
Outdated
@@ -240,6 +240,7 @@ def apply(self, func, *args, **kwargs) -> Series: | |||
Returns | |||
------- | |||
Series or DataFrame | |||
The resulting dtype will reflect the return value of the passed ``func``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem very clear to me. Why not something like A pandas object with the result of applying ``func`` to each group.
. Or maybe you can think of something better, but the return description should focus on what's being returned, not which type is being returned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense. Couldn't think of anything else so I use yours.
pandas/core/groupby/groupby.py
Outdated
@@ -1550,6 +1552,7 @@ def apply(self, func, *args, include_groups: bool = True, **kwargs) -> NDFrameT: | |||
Returns | |||
------- | |||
Series or DataFrame | |||
The resulting dtype will reflect the return value of the passed ``func``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
/preview |
Website preview of this PR available at: https://pandas.pydata.org/preview/pandas-dev/pandas/57862/ |
pandas/core/groupby/groupby.py
Outdated
being resampled: | ||
- pandas.api.typing.DatetimeIndexResamplerGroupby | ||
- pandas.api.typing.PeriodIndexResamplerGroupby | ||
- pandas.api.typing.TimedeltaIndexResamplerGroupby |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be rendering correctly: https://pandas.pydata.org/preview/pandas-dev/pandas/57862/docs/reference/api/pandas.core.groupby.DataFrameGroupBy.resample.html#pandas.core.groupby.DataFrameGroupBy.resample
Also, the type is incorrect now. Why don't you leave as type simply: DatetimeIndexResampler, PeriodIndexResampler or TimdeltaResampler
And in the description you can have something like Resampler object for the type of the index
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. But i'll hold this up and wait for the new flow regarding alll this doc-related thingy
@quangngd we changed how we specify the pending errors in |
@datapythonista resolved |
Thanks @quangngd |
…andas-dev#57862) * pandas.core.groupby.SeriesGroupBy.apply * pandas.core.groupby.GroupBy.apply * rm check * add a bunch * fix * fix * wip * update pandas.core.groupby.DataFrameGroupBy.resample * rm check from code_checls * minor
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.