Skip to content

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

Merged
merged 1 commit into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions pandas/tests/groupby/test_api_consistency.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

import inspect

import pytest

from pandas import (
DataFrame,
Series,
Expand All @@ -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
Copy link
Member

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 tested

Copy link
Member Author

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.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks (totally minor)

return

if groupby_func == "ngroup":
if groupby_func in ("cumcount", "ngroup"):
assert not hasattr(DataFrame, groupby_func)
return

Expand Down Expand Up @@ -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"):
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

@rhshadrach rhshadrach Mar 18, 2023

Choose a reason for hiding this comment

The 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

Expand Down
14 changes: 7 additions & 7 deletions pandas/tests/groupby/test_categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -1340,17 +1340,11 @@ def test_get_nonexistent_category():
)


def test_series_groupby_on_2_categoricals_unobserved(reduction_func, observed, request):
def test_series_groupby_on_2_categoricals_unobserved(reduction_func, observed):
# GH 17605
if reduction_func == "ngroup":
pytest.skip("ngroup is not truly a reduction")

if reduction_func == "corrwith": # GH 32293
mark = pytest.mark.xfail(
reason="TODO: implemented SeriesGroupBy.corrwith. See GH 32293"
)
request.node.add_marker(mark)

df = DataFrame(
{
"cat_1": Categorical(list("AABB"), categories=list("ABCD")),
Expand All @@ -1363,6 +1357,12 @@ def test_series_groupby_on_2_categoricals_unobserved(reduction_func, observed, r
expected_length = 4 if observed else 16

series_groupby = df.groupby(["cat_1", "cat_2"], observed=observed)["value"]

if reduction_func == "corrwith":
# TODO: implemented SeriesGroupBy.corrwith. See GH 32293
assert not hasattr(series_groupby, reduction_func)
return

agg = getattr(series_groupby, reduction_func)
result = agg(*args)

Expand Down
10 changes: 6 additions & 4 deletions pandas/tests/groupby/test_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -1483,14 +1483,16 @@ def test_numeric_only(kernel, has_arg, numeric_only, keys):
@pytest.mark.parametrize("dtype", [bool, int, float, object])
def test_deprecate_numeric_only_series(dtype, groupby_func, request):
# GH#46560
if groupby_func == "corrwith":
msg = "corrwith is not implemented on SeriesGroupBy"
request.node.add_marker(pytest.mark.xfail(reason=msg))

grouper = [0, 0, 1]

ser = Series([1, 0, 0], dtype=dtype)
gb = ser.groupby(grouper)

if groupby_func == "corrwith":
# corrwith is not implemented on SeriesGroupBy
assert not hasattr(gb, groupby_func)
return

method = getattr(gb, groupby_func)

expected_ser = Series([1, 0, 0])
Expand Down
16 changes: 8 additions & 8 deletions pandas/tests/groupby/transform/test_transform.py
Original file line number Diff line number Diff line change
Expand Up @@ -1125,9 +1125,9 @@ def test_transform_agg_by_name(request, reduction_func, frame_or_series):
g = obj.groupby(np.repeat([0, 1], 3))

if func == "corrwith" and isinstance(obj, Series): # GH#32293
request.node.add_marker(
pytest.mark.xfail(reason="TODO: implement SeriesGroupBy.corrwith")
)
# TODO: implement SeriesGroupBy.corrwith
assert not hasattr(g, func)
return

args = get_groupby_method_args(reduction_func, obj)
result = g.transform(func, *args)
Expand Down Expand Up @@ -1389,16 +1389,16 @@ def test_null_group_str_transformer(request, dropna, transformation_func):


def test_null_group_str_reducer_series(request, dropna, reduction_func):
# GH 17093
if reduction_func == "corrwith":
msg = "corrwith not implemented for SeriesGroupBy"
request.node.add_marker(pytest.mark.xfail(reason=msg))

# GH 17093
index = [1, 2, 3, 4] # test transform preserves non-standard index
ser = Series([1, 2, 2, 3], index=index)
gb = ser.groupby([1, 1, np.nan, np.nan], dropna=dropna)

if reduction_func == "corrwith":
# corrwith not implemented for SeriesGroupBy
assert not hasattr(gb, reduction_func)
return

args = get_groupby_method_args(reduction_func, ser)

# Manually handle reducers that don't fit the generic pattern
Expand Down