Skip to content

TST: check inequality by comparing categorical with NaN ( #28384 ) #36520

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 5 commits into from
Sep 22, 2020

Conversation

junjunjunk
Copy link
Contributor

@junjunjunk junjunjunk commented Sep 21, 2020

@junjunjunk
Copy link
Contributor Author

Please review.

And I need some help.
I don't know why CI is failing.From the looks of it, it looks like it's timed out.

@@ -148,3 +148,13 @@ def test_use_inf_as_na_outside_context(self, values, expected):
result = pd.isna(DataFrame(cat))
expected = DataFrame(expected)
tm.assert_frame_equal(result, expected)

def test_compare_unequal(self):
Copy link
Member

Choose a reason for hiding this comment

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

test_compare_categorical_with_missing

Copy link
Contributor Author

@junjunjunk junjunjunk Sep 21, 2020

Choose a reason for hiding this comment

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

Thank you. Fixed.
c64a8d3

Copy link
Member

@arw2019 arw2019 left a comment

Choose a reason for hiding this comment

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

I think possibly add a non-categorical example since the OP was about the difference in behavior between categorical vs. non-categorical. See #28384 (comment)

Re: CI failures those are unrelated I think. Merge with master and they'll be rerun

@junjunjunk
Copy link
Contributor Author

I think possibly add a non-categorical example since the OP was about the difference in behavior between categorical vs. non-categorical. See #28384 (comment)

Re: CI failures those are unrelated I think. Merge with master and they'll be rerun

Thank you for your input. I added test case.
And I rebased my branch.

@@ -148,3 +148,22 @@ def test_use_inf_as_na_outside_context(self, values, expected):
result = pd.isna(DataFrame(cat))
expected = DataFrame(expected)
tm.assert_frame_equal(result, expected)

def test_compare_categorical_with_missing(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

can you fully replicate the tests in the OP, e.g. add for == as well.

ideally parameterize this over these 4 cases (or 2 and 2 in-line if easier).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for review. Done.

@jreback jreback added Categorical Categorical Data Type Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Testing pandas testing functions or related to the test suite labels Sep 22, 2020
@jreback jreback added this to the 1.2 milestone Sep 22, 2020
@jreback jreback merged commit 3295270 into pandas-dev:master Sep 22, 2020
@jreback
Copy link
Contributor

jreback commented Sep 22, 2020

thanks @junjunjunk

@junjunjunk junjunjunk deleted the add_test branch September 30, 2020 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Categorical Categorical Data Type Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Categorical equality with NaN behaves unexpectedly
3 participants