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.

Sign up for GitHub

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

Fix xfail incompatibilities #12423

Merged
merged 3 commits into from
Dec 20, 2022
Merged

Fix xfail incompatibilities #12423

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e01e9c6
Use applymarker to only xfail the cases that should actually fail.
vyasr Dec 19, 2022
451115c
Remove outdated comment.
vyasr Dec 19, 2022
9eedf97
Make the xfail conditional.
vyasr Dec 19, 2022
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
34 changes: 17 additions & 17 deletions python/cudf/cudf/tests/test_dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2349,17 +2349,6 @@ def test_comparison_binops_df(pdf, gdf, binop, other):


@pytest_unmark_spilling
@pytest.mark.xfail(
reason="""
Currently we will not match pandas for equality/inequality operators when
there are columns that exist in a Series but not the DataFrame because
pandas returns True/False values whereas we return NA. However, this
reindexing is deprecated in pandas so we opt not to add support. This test
should start passing once pandas removes the deprecated behavior in 2.0.
When that happens, this test can be merged with the two tests above into a
single test with common parameters.
"""
)
@pytest.mark.parametrize(
"binop",
[
Expand All @@ -2381,7 +2370,7 @@ def test_comparison_binops_df(pdf, gdf, binop, other):
pd.Series([1.0, 2.0, 3.0], index=["x", "y", "z"]),
],
)
def test_comparison_binops_df_reindexing(pdf, gdf, binop, other):
def test_comparison_binops_df_reindexing(request, pdf, gdf, binop, other):
# Avoid 1**NA cases: https://github.com/pandas-dev/pandas/issues/29997
pdf[pdf == 1.0] = 2
gdf[gdf == 1.0] = 2
Expand All @@ -2401,6 +2390,22 @@ def test_comparison_binops_df_reindexing(pdf, gdf, binop, other):
compare_error_message=False,
)
else:
request.applymarker(
pytest.mark.xfail(
condition=pdf.columns.difference(other.index).size > 0,
reason="""
Currently we will not match pandas for equality/inequality
operators when there are columns that exist in a Series but not
the DataFrame because pandas returns True/False values whereas
we return NA. However, this reindexing is deprecated in pandas
so we opt not to add support. This test should start passing
once pandas removes the deprecated behavior in 2.0. When that
happens, this test can be merged with the two tests above into
a single test with common parameters.
""",
)
)

if isinstance(other, (pd.Series, pd.DataFrame)):
other = cudf.from_pandas(other)
g = binop(gdf, other)
Expand Down Expand Up @@ -5181,11 +5186,6 @@ def test_rowwise_ops_datetime_dtypes(data, op, skipna):

pdf = gdf.to_pandas()

# TODO: This behavior seems erroneous in pandas. Why is the min/max over
# a mix of datetime and numeric dtypes not just throwing an error? This
# test will have to be rewritten anyway in pandas 2.0 when the implicit
# numeric_only behavior changes, at which point the dtype mixing should be
# reconsidered as well.
with expect_warning_if(
not all(cudf.api.types.is_datetime64_dtype(dt) for dt in gdf.dtypes),
UserWarning,
Expand Down