-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: Remove some xfails in groupby tests #52065
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,6 @@ | |
|
||
import inspect | ||
|
||
import pytest | ||
|
||
from pandas import ( | ||
DataFrame, | ||
Series, | ||
|
@@ -16,16 +14,13 @@ | |
) | ||
|
||
|
||
def test_frame_consistency(request, groupby_func): | ||
def test_frame_consistency(groupby_func): | ||
# GH#48028 | ||
if groupby_func in ("first", "last"): | ||
msg = "first and last are entirely different between frame and groupby" | ||
request.node.add_marker(pytest.mark.xfail(reason=msg)) | ||
if groupby_func in ("cumcount",): | ||
msg = "DataFrame has no such method" | ||
request.node.add_marker(pytest.mark.xfail(reason=msg)) | ||
# first and last are entirely different between frame and groupby | ||
return | ||
|
||
if groupby_func == "ngroup": | ||
if groupby_func in ("cumcount", "ngroup"): | ||
assert not hasattr(DataFrame, groupby_func) | ||
return | ||
|
||
|
@@ -82,13 +77,10 @@ def test_frame_consistency(request, groupby_func): | |
def test_series_consistency(request, groupby_func): | ||
# GH#48028 | ||
if groupby_func in ("first", "last"): | ||
msg = "first and last are entirely different between Series and groupby" | ||
request.node.add_marker(pytest.mark.xfail(reason=msg)) | ||
if groupby_func in ("cumcount", "corrwith"): | ||
msg = "Series has no such method" | ||
request.node.add_marker(pytest.mark.xfail(reason=msg)) | ||
# first and last are entirely different between Series and groupby | ||
return | ||
|
||
if groupby_func == "ngroup": | ||
if groupby_func in ("cumcount", "corrwith", "ngroup"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ive had these corrwith xfails on my radar for a bit. if we remove the xfail, should we also say "no" to #32293? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so; replacing xfail with the attribute check means that this test will still start failing if they are ever added, but without the perf impact of using an xfail. And it is better behaved - the xfail can pass even if corrwith is added (if it doesn't happen to pass the rest of this test). |
||
assert not hasattr(Series, groupby_func) | ||
return | ||
|
||
|
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.
For these cases where nothing is tested, I would prefer using
pytest.skip(reason)
just so it's clear that nothing is supposed to be testedThere 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.
Makes sense - I'll do a follow up.
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 (totally minor)