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

Adjust groupby tests for string option #56414

Closed
wants to merge 21 commits into from

Conversation

phofl
Copy link
Member

@phofl phofl commented Dec 8, 2023

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@phofl phofl requested a review from mroeschke December 8, 2023 21:43
Comment on lines +714 to +715
or isinstance(nulls_fixture, Decimal)
and Decimal.is_nan(nulls_fixture)
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.

Could you add parentheses around the last two conditions here for clarify (I think it's the same behavior)

@@ -707,7 +709,15 @@ def test_first_multi_key_groupby_categorical():
@pytest.mark.parametrize("method", ["first", "last", "nth"])
def test_groupby_last_first_nth_with_none(method, nulls_fixture):
# GH29645
expected = Series(["y"])
if nulls_fixture is not pd.NA and (
Copy link
Member

Choose a reason for hiding this comment

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

I think nulls_fixture is not pd.NA is unnecessary

@@ -171,7 +171,9 @@ def test_groupby_quantile_with_arraylike_q_and_int_columns(frame_size, groupby,
def test_quantile_raises():
df = DataFrame([["foo", "a"], ["foo", "b"], ["foo", "c"]], columns=["key", "val"])

with pytest.raises(TypeError, match="cannot be performed against 'object' dtypes"):
with pytest.raises(
TypeError, match="cannot be performed against 'object' dtypes|No matching"
Copy link
Member

Choose a reason for hiding this comment

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

What's the full error message for the No matching case?

@@ -37,7 +37,7 @@ def store(group):
tm.assert_frame_equal(groups[0], expected_value)


def test_apply_index_date():
def test_apply_index_date(using_infer_string):
Copy link
Member

Choose a reason for hiding this comment

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

This one looks unused?

# multiple groupers, don't re-expand the output space
# of the grouper
# gh-14942 (implement)
# gh-10132 (back-compat)
# gh-8138 (back-compat)
# gh-8869

if not observed and using_infer_string:
mark = pytest.mark.xfail(reason="fill_value=0 invalid for string dtype")
Copy link
Member

Choose a reason for hiding this comment

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

Should this test be fixed in the future? If so, okay to xfail - otherwise I'd prefer to test for the exception with pytest.raises. xfails have a perf impact on running the tests.

pandas/tests/groupby/test_groupby.py Show resolved Hide resolved
@@ -844,7 +844,7 @@ def test_groupby_level_index_value_all_na(self):
columns=["C"],
dtype="int64",
)
tm.assert_frame_equal(result, expected)
tm.assert_frame_equal(result, expected, check_index_type=not using_infer_string)
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 just change dytpe="object" on L840?

@mroeschke mroeschke added Testing pandas testing functions or related to the test suite Strings String extension data type and string data labels Jan 8, 2024
Copy link
Contributor

github-actions bot commented Feb 8, 2024

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Feb 8, 2024
@mroeschke
Copy link
Member

Is this still needed?

@phofl
Copy link
Member Author

phofl commented Mar 16, 2024

yep, I'll update

…groupby

# Conflicts:
#	pandas/tests/groupby/test_apply.py
#	pandas/tests/groupby/test_groupby.py
#	pandas/tests/groupby/test_numeric_only.py
#	pandas/tests/groupby/test_raises.py
#	pandas/tests/groupby/test_reductions.py
@mroeschke
Copy link
Member

Closing to clear the queue, but feel free to update and reopen anytime

@mroeschke mroeschke closed this Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Strings String extension data type and string data Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants