Skip to content

TST: Use stronger assertion in pandas/tests/groupby/test_allowlist.py::test_regression_allowlist_methods #49629

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

Closed
mroeschke opened this issue Nov 10, 2022 · 5 comments · Fixed by #49755
Labels
Groupby Testing pandas testing functions or related to the test suite

Comments

@mroeschke
Copy link
Member

Before #49611, test_regression_allowlist_methods was a comprehensive groupby test that asserted frame.groupby(...).op(...) was equivalent to frame.op(level=..., ...). Now that level has been removed, this test should ideally have a stronger assertion than current testing that a DataFrame is returned; ideally using tm.assert_frame_equal

@mroeschke mroeschke added Testing pandas testing functions or related to the test suite Groupby labels Nov 10, 2022
@isaac-chung
Copy link
Contributor

Is this an issue that's welcoming contributions?
If yes, I'd love to give it a go.

@mroeschke
Copy link
Member Author

Definitely @isaac-chung. Feel free to tackle it!

@isaac-chung
Copy link
Contributor

Looking for ways to generate the expected frame to use tm.assert_frame_equal. Trying to get some context for the removal of level from this PR and this suggestion. It seems that the previous way of building the expected frame is no longer supported.

Is this something close to the best we can do for now? Even then, this does not match result currently when given sort=True or axis=1.

expected = frame.groupby(level="first").apply(lambda h: getattr(h, op)(axis=axis))

I hope to see if there is something obvious that I'm missing before I go into a rabbit hole. Also happy to discuss this or let another contributor take a go if needed.

@mroeschke
Copy link
Member Author

The apply idea is a good one - fair point though it might not conveniently support all the supported arguments.

Since this test is mainly smoketest for groupby methods and args, I think it's valid to replace raw_frame with a "simple DataFrame" like DataFrame([0]) that should hopefully have similar results for a lot of the groupby methods and their args.

@isaac-chung
Copy link
Contributor

isaac-chung commented Nov 17, 2022

Sounds good, this would simplify things quite well. Let me share what I currently have in a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groupby Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants