Skip to content

[SPARK-58766][PYTHON][TEST][FOLLOWUP] Render NaT-sentinel time64 scalars in golden files - #58266

Closed
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:golden-nat-sentinel
Closed

[SPARK-58766][PYTHON][TEST][FOLLOWUP] Render NaT-sentinel time64 scalars in golden files#58266
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:golden-nat-sentinel

Conversation

@Spenserrrr

@Spenserrrr Spenserrrr commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add a ValueError branch to _scalar_str in python/pyspark/testing/goldenutils.py so a time64[ns] scalar holding INT64_MIN renders as the marker NaT collision, beside the existing temporal overflow and raw-bytes fallbacks, and regenerate the four affected from_pandas type-scalar golden cells (int64:standard and Int64:standard x time64[ns], safe and unsafe).

Rendering a time64[ns] scalar goes through pandas, because Python's datetime.time has only microsecond resolution. pandas reserves INT64_MIN as its NaT sentinel, so it refuses that one value with ValueError: NaTType does not support time. The branch is gated on both the type and the value, so any other ValueError still propagates.

Why are the changes needed?

PyArrowArrayFromPandasTypeScalarTests currently fails on master. The four cells above read ERR@ValueError, which by convention means the conversion raised — but the conversion succeeds and only the rendering failed. The broad try/except in the cell helper recorded the rendering failure as if it were a conversion error, so narrowing that try/except in #58091 turned the mislabelling into a test error.

Does this PR introduce any user-facing change?

No. Test-only.

How was this patch tested?

python/pyspark/tests/upstream/pyarrow/ passes (63 passed, 1 skipped), and a sweep over pyarrow 18-25 x pandas 2/3 is 16/16 with no overrides entries.

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

Generated-by: Claude Code (Claude Opus 5)

…ars in golden files

Rendering a `time64[ns]` scalar goes through pandas, since Python's `time` has
only microsecond resolution, and pandas reads INT64_MIN as its NaT sentinel and
refuses that one value. `_scalar_str` now records it as `NaT collision`, beside
the existing `temporal overflow` and raw-bytes fallbacks, and the four affected
type-scalar golden cells are regenerated.

Those cells previously read `ERR@ValueError`, which claims the conversion
raised; the conversion in fact succeeds and only the rendering failed. The
broad cell try/except recorded that as if it were a conversion error, and
narrowing it in apache#58091 turned the mislabelling into a test error.
@Spenserrrr
Spenserrrr marked this pull request as ready for review August 25, 2026 02:36
@Spenserrrr
Spenserrrr marked this pull request as draft August 25, 2026 02:48
@Spenserrrr
Spenserrrr marked this pull request as ready for review August 25, 2026 03:58
@Spenserrrr

Copy link
Copy Markdown
Contributor Author

Hi @zhengruifeng @Yicong-Huang! Could you take a look at this when you have time? It fixes PyArrowArrayFromPandasTypeScalarTests, which currently fails on master, so it may surface on other PRs' CI too. My #58091 exposed it, and I wrote the reason in the PR description. CI here is green, including the pyspark-core shard that's the one failing on master. Thanks!

@zhengruifeng zhengruifeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

0 blocking, 0 non-blocking, 0 nits.
No findings; the fallback is narrowly scoped and the golden updates match the exercised matrix cells.

Verification

I traced the from_pandas test helper and confirmed ERR cells are produced only when pa.Array.from_pandas raises, before result rendering. I also checked that the type-scalar matrix has one time64[ns] target and the changed int64/Int64 source rows, then locally evaluated the renderer for pa.time64('ns') with pandas' NaT sentinel; it produces the regenerated NaT collision cell.

zhengruifeng pushed a commit that referenced this pull request Aug 25, 2026
…ars in golden files

### What changes were proposed in this pull request?

Add a `ValueError` branch to `_scalar_str` in `python/pyspark/testing/goldenutils.py` so a `time64[ns]` scalar holding INT64_MIN renders as the marker `NaT collision`, beside the existing `temporal overflow` and raw-bytes fallbacks, and regenerate the four affected `from_pandas` type-scalar golden cells (`int64:standard` and `Int64:standard` x `time64[ns]`, safe and unsafe).

Rendering a `time64[ns]` scalar goes through pandas, because Python's `datetime.time` has only microsecond resolution. pandas reserves INT64_MIN as its NaT sentinel, so it refuses that one value with `ValueError: NaTType does not support time`. The branch is gated on both the type and the value, so any other `ValueError` still propagates.

### Why are the changes needed?

`PyArrowArrayFromPandasTypeScalarTests` currently fails on master. The four cells above read `ERRValueError`, which by convention means the conversion raised — but the conversion succeeds and only the rendering failed. The broad `try/except` in the cell helper recorded the rendering failure as if it were a conversion error, so narrowing that `try/except` in #58091 turned the mislabelling into a test error.

### Does this PR introduce _any_ user-facing change?

No. Test-only.

### How was this patch tested?

`python/pyspark/tests/upstream/pyarrow/` passes (63 passed, 1 skipped), and a sweep over pyarrow 18-25 x pandas 2/3 is 16/16 with no `overrides` entries.

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

Generated-by: Claude Code (Claude Opus 5)

Closes #58266 from Spenserrrr/golden-nat-sentinel.

Authored-by: Spenser Sun <hsun112358@gmail.com>
Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>
(cherry picked from commit 08e97de)
Signed-off-by: Ruifeng Zheng <ruifengz@foxmail.com>
@zhengruifeng

Copy link
Copy Markdown
Contributor

Merge Summary:

Posted by merge_spark_pr.py

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.

2 participants