-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: fix ngroups and len(groups) inconsistency when using [Grouper(freq=)] (GH33132) #33135
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
Conversation
@@ -1863,6 +1863,17 @@ def test_groupby_groups_in_BaseGrouper(): | |||
expected = df.groupby(["beta", "alpha"]) | |||
assert result.groups == expected.groups | |||
|
|||
# GH 33132 |
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.
can you make this a separate test
result = df.groupby(["beta", pd.Grouper(level="alpha", freq='D')]) | ||
assert result.ngroups == len(result) | ||
|
||
result = df.groupby([pd.Grouper(level="alpha", freq='D'), "beta"]) |
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.
linter is going to want double-quotes on 'D'
@@ -837,11 +838,22 @@ def names(self): | |||
|
|||
@property | |||
def groupings(self) -> "List[grouper.Grouping]": | |||
codes, _, _ = self.group_info |
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.
I am not really sure what you are hoping to accomplish here.
] | ||
|
||
def __iter__(self): |
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.
the issue then len(Grouper) != ngroups right? why are you adding __iter__
? rather than for example overriding __len__
?
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.
Indeed changing __iter__
was a bad idea since this changes the behavior of resample. Anyway, iterating over a nested BinGrouper
is somewhat part of the problem (check #33132 (comment) plz). #33303 probably has a better approach to resolve this.
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.
see comment
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff