GH203 Split groupby with as_index (temptative) #1014
Conversation
|
I am still having overlap issues in the overload but looking into it, feel free to suggest something else. |
There was a problem hiding this comment.
Good start, but I think you will have to work on the following:
- Have to follow the suggestion at BUG: df.groupby(col, as_index=False).value_counts() returns a DataFrame but is annotated as Series #203 (comment) , item 2, to add a
Genericparameter toDataFrameGroupBythat essentially represents whetheras_indexisTrueorFalse - You will have to do the same separation on the overloads for
groupby()with the otherbyarguments, and add appropriate tests.
tests/test_frame.py
Outdated
| @@ -1048,7 +1066,7 @@ def test_types_groupby() -> None: | |||
|
|
|||
| df1: pd.DataFrame = df.groupby(by="col1").agg("sum") | |||
| df2: pd.DataFrame = df.groupby(level="ind").aggregate("sum") | |||
| df3: pd.DataFrame = df.groupby(by="col1", sort=False, as_index=True).transform( | |||
| df3: pd.Series = df.groupby(by="col1", sort=False, as_index=True).transform( | |||
There was a problem hiding this comment.
Given the nature of this change, can you change this test to use the check(assert_type(... pattern?
We have old tests that haven't been converted - this is a good opportunity to convert them.
There was a problem hiding this comment.
Agreed, I will tackle those once we agree on the potential solution so I can do one thing at a time not to confuse the amount of code to review too much.
|
Seems like this is going to take a little longer, I think I understand your suggestion in #203, let me try to come up with something. |
|
So I am fine with this. Do you want me to merge it so that |
|
I will open another issue to track down the other methods, would rather merge small changes than one large PR. |
assert_type()to assert the type of any return value