Skip to content

[SPARK-59514][PYTHON] Handle nonfinite values in assertDataFrameEqual - #58790

Open
laserninja wants to merge 1 commit into
apache:masterfrom
laserninja:fix/nonfinite-comparison
Open

laserninja wants to merge 1 commit into
apache:masterfrom
laserninja:fix/nonfinite-comparison

Conversation

@laserninja

@laserninja laserninja commented Sep 14, 2026

Copy link
Copy Markdown

What changes were proposed in this pull request?

Jira: SPARK-59514

Compare NaN and infinity explicitly before applying finite-value tolerances. Preserve matching NaNs and same-sign infinities, and add tests for both argument directions, zero/default tolerances, and nested values.

Why are the changes needed?

The tolerance inequality does not reject NaN arithmetic, and an infinite expected value can make both sides infinity. As a result, NaN versus a finite value and opposite-sign infinities can pass equality assertions.

from pyspark.sql import Row
from pyspark.testing import assertDataFrameEqual
assertDataFrameEqual([Row(x=float("nan"))], [Row(x=1.0)])
assertDataFrameEqual([Row(x=1.0)], [Row(x=float("inf"))])
assertDataFrameEqual([Row(x=float("inf"))], [Row(x=float("-inf"))])
# All three return without raising

Does this PR introduce any user-facing change?

Yes. Unequal special values now raise DIFFERENT_ROWS. Matching NaNs and same-sign infinities continue to compare equal, and finite-value tolerance behavior is unchanged.

How was this patch tested?

The new regression tests failed before the fix and passed afterward on Python 3.10.11 with NumPy 2.2.6. The focused run passed 3 tests, including parameterized subtests:

PYTHONPATH=python TERM=dumb python -m unittest pyspark.sql.tests.test_utils.NonFiniteComparisonTests -q
PYTHON_EXECUTABLE=python ./dev/lint-python --compile --custom-pyspark-error --ruff

Run the commands with the Python virtual environment activated; the local run shared an environment from a sibling checkout. The tests call public Python APIs directly and require no SparkSession. Python compilation, custom-error checks, Ruff lint, and Ruff formatting passed. JVM-backed suites were not run because these fixes are confined to Python-side logic.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: OpenAI Codex (GPT-6).

@HyukjinKwon

Copy link
Copy Markdown
Member

Review — [PYTHON] Handle nonfinite values in assertDataFrameEqual

LGTM. The fix in compare_vals is correct. Previously the float branch only did abs(val1 - val2) > (atol + rtol * abs(val2)); since abs(nan - x) is nan and nan > y is False, a NaN silently compared equal to any finite value. The new guards make NaN equal only to NaN, and inf/-inf compare by exact equality. The early returns don't skip any tail logic — the finite path's only other exit is return False, otherwise it falls through to the same trailing return True. Because both list and DataFrame inputs flow through compare_vals, the fix covers both.

I exercised the behavior directly (list inputs need no JVM): NaN vs finite/inf → unequal; NaN/inf/-inf vs themselves → equal; rtol tolerance and nested Row/list/map cases all behave as asserted.

The tests are well chosen — both argument orders, checkRowOrder, tolerance settings, and nested structures — and NonFiniteComparisonTests(unittest.TestCase) is the right minimal base since the list inputs require no SparkSession.

Optional nit: the title has no [SPARK-xxxxx] id. For a behavior bug fix Spark usually wants a JIRA ticket unless it is intentionally kept trivial/minor.

@laserninja
laserninja force-pushed the fix/nonfinite-comparison branch from 0b778af to a6ec0f2 Compare September 15, 2026 01:16
@laserninja laserninja changed the title [PYTHON] Handle nonfinite values in assertDataFrameEqual [SPARK-59514][PYTHON] Handle nonfinite values in assertDataFrameEqual Sep 15, 2026
@laserninja

Copy link
Copy Markdown
Author

Thanks for the review and verification. Created SPARK-59514, linked it in the description, and added the Jira ID to the title. No code changes were needed for the review item.

@laserninja
laserninja marked this pull request as ready for review September 15, 2026 02:32
@uros-b
uros-b requested a review from HyukjinKwon September 15, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants