Skip to content
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

TST: Added a test for groupby.prod #56384

Merged
merged 8 commits into from
Dec 9, 2023
Merged

Conversation

ccccjone
Copy link
Contributor

@ccccjone ccccjone commented Dec 7, 2023

@ccccjone ccccjone changed the title Added a test for groupby.prod TST: Added a test for groupby.prod Dec 8, 2023
@ccccjone ccccjone requested a review from phofl December 8, 2023 20:45
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

pandas/tests/groupby/test_reductions.py Outdated Show resolved Hide resolved
[1, multiplier],
]
df = DataFrame(data, columns=["A", "B"])
df_prod = DataFrame([df.prod()], columns=df.columns)
Copy link
Member

Choose a reason for hiding this comment

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

Can you hardcode this instead (assuming the parametrization will be removed)

Copy link
Member

@rhshadrach rhshadrach Dec 9, 2023

Choose a reason for hiding this comment

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

By "hardcode", I mean something like expected = DataFrame([[12345], [6789]], columns["B"]). In other words, don't use df.prod().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rhshadrach Sorry for the misunderstanding. I have now updated the code based on your explanation. Could you please take a look and let me know if this is now correct, or if there are any further modifications needed? Thanks for your kind assistance.

pandas/tests/groupby/test_reductions.py Outdated Show resolved Hide resolved
pandas/tests/groupby/test_reductions.py Show resolved Hide resolved
@ccccjone
Copy link
Contributor Author

ccccjone commented Dec 9, 2023

Hi @rhshadrach , thank you for your review and suggestions. I have updated the code based on your feedback. Please review. Appreciate your help and guidance!

@ccccjone ccccjone requested a review from rhshadrach December 9, 2023 00:56
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm

@rhshadrach rhshadrach added Bug Groupby Reduction Operations sum, mean, min, max, etc. Needs Tests Unit test(s) needed to prevent regressions labels Dec 9, 2023
@rhshadrach rhshadrach added this to the 2.2 milestone Dec 9, 2023
@rhshadrach rhshadrach merged commit ebde354 into pandas-dev:main Dec 9, 2023
56 of 58 checks passed
@rhshadrach
Copy link
Member

Thanks @ccccjone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Groupby Needs Tests Unit test(s) needed to prevent regressions Reduction Operations sum, mean, min, max, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: prod and groupby.prod behave differently
3 participants