Skip to content

Added pd.NA to nulls_fixture #31799

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 6 commits into from
Feb 20, 2020
Merged

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Feb 8, 2020

@jorisvandenbossche

I think we should do this. Simply xfailed issues up for discussion on how to resolve

@@ -1964,6 +1967,9 @@ def test_isin_nan_common_float64(self, nulls_fixture):
pytest.skip("pd.NaT not compatible with Float64Index")

# Float64Index overrides isin, so must be checked separately
if nulls_fixture is pd.NA:
pytest.xfail("Not sure how to handle with Float64Index?")
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about this test. Maybe logical to support, though we also skip for pd.NaT atm

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should support converting pd.NA into np.nan in creating a float index, since pd.NA and np.nan behave differently

@jorisvandenbossche
Copy link
Member

Cool! Yes, I agree it's a good idea to add it, and skip for now where it is not yet supported

@jbrockmendel
Copy link
Member

should this use the dynamic-xfail pattern we've used elsewhere?

+1 on the idea

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

lgtm (comment). ping when ready.

@@ -129,6 +129,10 @@ def test_compare_scalar_interval_mixed_closed(self, op, closed, other_closed):
def test_compare_scalar_na(self, op, array, nulls_fixture):
result = op(array, nulls_fixture)
expected = self.elementwise_comparison(op, array, nulls_fixture)

if nulls_fixture is pd.NA:
pytest.xfail("assert_numpy_array_equal broken for pd.NA")
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 make an issue for this (and ref here)

@jreback jreback added Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Testing pandas testing functions or related to the test suite labels Feb 9, 2020
@jreback jreback added this to the 1.1 milestone Feb 9, 2020
@WillAyd WillAyd marked this pull request as ready for review February 11, 2020 16:31
@WillAyd
Copy link
Member Author

WillAyd commented Feb 11, 2020

Opened issues where I think applicable and updated comments accordingly

@WillAyd
Copy link
Member Author

WillAyd commented Feb 20, 2020

Any other feedback here?

@jreback
Copy link
Contributor

jreback commented Feb 20, 2020

this is ok

Assume u have opened an issue for the xfail u r adding

@WillAyd
Copy link
Member Author

WillAyd commented Feb 20, 2020

Noted in tests - they are #31884 #31883 and #31881

@WillAyd WillAyd merged commit 304209f into pandas-dev:master Feb 20, 2020
@WillAyd
Copy link
Member Author

WillAyd commented Feb 20, 2020

Thanks all for feedback

roberthdevries pushed a commit to roberthdevries/pandas that referenced this pull request Mar 2, 2020
@WillAyd WillAyd deleted the na-nulls-fixture branch April 12, 2023 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants