-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: propagate dropna in pd.Grouper #36604
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
ae57e06
to
ff6e013
Compare
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.
lgtm, one small request.
def test_Grouper_dropna_propagation(dropna): | ||
df = pd.DataFrame({"A": [0, 0, 1, None], "B": [1, 2, 3, None]}) | ||
gb = df.groupby("A", dropna=dropna) | ||
assert gb.grouper.dropna is dropna |
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 think use == instead of is here.
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.
Also, lowercase g and lets add the PR number here.
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 looking this over! Fixed both of these
I noticed that the I said |
The test is fine it was a general question in the PR. Was addressed by Jeff as well so otherwise lgtm
…Sent from my iPhone
On Sep 25, 2020, at 11:49 AM, Richard Shadrach ***@***.***> wrote:
@rhshadrach commented on this pull request.
In pandas/tests/groupby/test_groupby_dropna.py:
> @@ -162,6 +162,13 @@ def test_groupby_dropna_series_by(dropna, expected):
tm.assert_series_equal(result, expected)
***@***.***("dropna", (False, True))
+def test_grouper_dropna_propagation(dropna):
+ df = pd.DataFrame({"A": [0, 0, 1, None], "B": [1, 2, 3, None]})
+ gb = df.groupby("A", dropna=dropna)
+ assert gb.grouper.dropna == dropna
@WillAyd maybe you're thinking we should only be testing the public API and not any internals?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
ok this is fine |
@@ -97,6 +98,7 @@ def __init__( | |||
self.group_keys = group_keys | |||
self.mutated = mutated | |||
self.indexer = indexer | |||
self.dropna = dropna |
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.
@arw2019 this attribute isnt used anywhere except for in the test added in this PR. is it still needed? is it part of a precursor to something on the horizon?
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
A precursor to #35751