Skip to content

TST: added test for missing categories for value_counts #54837

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 8 commits into from
Aug 31, 2023

Conversation

rsm-23
Copy link
Contributor

@rsm-23 rsm-23 commented Aug 29, 2023

@rsm-23
Copy link
Contributor Author

rsm-23 commented Aug 29, 2023

@rhshadrach can you please review this PR?

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!

@rsm-23 rsm-23 requested a review from rhshadrach August 29, 2023 11:24
@mroeschke mroeschke added the Testing pandas testing functions or related to the test suite label Aug 29, 2023
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 update! A small request to make the test a little simpler.

Comment on lines 198 to 206
expected = pd.Series(
1,
index=pd.MultiIndex.from_arrays(
[pd.CategoricalIndex([1, 2, 4], categories=[1, 2, 3, 4], name="a")]
),
name="count",
)
# result should include the missing category
expected[3] = 0
Copy link
Member

Choose a reason for hiding this comment

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

Instead of modifying expected in the last line, it is slightly simpler to specify the correct results up front.

    expected = pd.Series(
        [1, 1, 1, 0],
        index=pd.MultiIndex.from_arrays(
            [pd.CategoricalIndex([1, 2, 4, 3], categories=[1, 2, 3, 4], name="a")]
        ),
        name="count",
    )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @rhshadrach . I have made the change.

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 can you please take a look?

@rsm-23 rsm-23 requested a review from rhshadrach August 30, 2023 20:22
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 Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Categorical Categorical Data Type labels Aug 31, 2023
@rhshadrach rhshadrach added this to the 2.2 milestone Aug 31, 2023
@rhshadrach rhshadrach added Needs Tests Unit test(s) needed to prevent regressions and removed Testing pandas testing functions or related to the test suite labels Aug 31, 2023
@rhshadrach rhshadrach merged commit 93359af into pandas-dev:main Aug 31, 2023
@rhshadrach
Copy link
Member

Thanks @rsm-23

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff Categorical Categorical Data Type Needs Tests Unit test(s) needed to prevent regressions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TST: Add tests for DataFrame/Series.value_counts with missing categories
3 participants