Skip to content

[SPARK-54946][PYTHON][TEST] Add tests for pa.Array.to_pandas with coerce_temporal_nanoseconds#57435

Open
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:coerce-temporal-nanoseconds-tests
Open

[SPARK-54946][PYTHON][TEST] Add tests for pa.Array.to_pandas with coerce_temporal_nanoseconds#57435
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:coerce-temporal-nanoseconds-tests

Conversation

@Spenserrrr

@Spenserrrr Spenserrrr commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This PR adds a golden-file test that pins the behavior of PyArrow's Array.to_pandas(coerce_temporal_nanoseconds=True) across Arrow data types.

  • New test: python/pyspark/tests/upstream/pyarrow/test_pyarrow_arrow_to_pandas_coerce_temporal.py
  • New golden files: golden_pyarrow_arrow_to_pandas_coerce_temporal.{csv,md}
  • Registered the module in dev/sparktestsupport/modules.py.

The test follows the same pattern as test_pyarrow_arrow_to_pandas_default (SPARK-54944). The only behavioral difference is that each source array is converted with arr.to_pandas(coerce_temporal_nanoseconds=True). The source arrays are limited to the temporal types the argument affects (timestamp and duration in units
s/ms/us/ns, tz-aware timestamp, date, and time), and an overflow case and a few non-temporal control rows (int64/float64/string) to demonstrate the argument leaves non-temporal types unaffected.

The golden files record the resulting pandas Series dtype and values. They were generated with pandas 2.3.3, pyarrow 24.0.0, and numpy 2.4.1. Notable behavior:

  • timestamp[s|ms|us|ns] all coerce to datetime64[ns].
  • tz-aware timestamp[us, tz=UTC] coerces to datetime64[ns, UTC].
  • duration[s|ms|us|ns] all coerce to timedelta64[ns].
  • date32/date64 and time32/time64 remain object dtype
  • A far-future second-resolution timestamp (year 2500) that cannot fit in the nanosecond range raises ArrowInvalid.

Why are the changes needed?

This is part of SPARK-54936 ("Monitor behaviour changes from upstream").

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New golden-file test. Generated the golden files with
SPARK_GENERATE_GOLDEN_FILES=1 (pandas 2.3.3, pyarrow 24.0.0, numpy 2.4.1),
reviewed every cell, then confirmed the test passes in comparison mode:

python -m pytest
python/pyspark/tests/upstream/pyarrow/test_pyarrow_arrow_to_pandas_coerce_temporal.py

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

Generated-by: Claude Code (Opus 4.8)

@Spenserrrr

Spenserrrr commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@Yicong-Huang

@Spenserrrr
Spenserrrr force-pushed the coerce-temporal-nanoseconds-tests branch from 2a67377 to 77d4b15 Compare July 22, 2026 20:31
uros-b
uros-b reviewed Jul 22, 2026
View reviewed changes

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[SPARK-54946][PYTHON][TEST] Add tests for pa.Array.to_pandas with coe…

The PR title looks truncated @Spenserrrr

@uros-b

uros-b commented Jul 22, 2026

Copy link
Copy Markdown
Member

Also, could we please add date32/date64 cases using date_as_object=False? With the default True, these rows remain object dtype, so they do not test the date conversion behavior of coerce_temporal_nanoseconds=True.

@uros-b

uros-b commented Jul 22, 2026

Copy link
Copy Markdown
Member

Also, could we please add a duration[s]:overflow case parallel to timestamp[s]:overflow? Coercing a duration larger than roughly 292 years to nanoseconds also exceeds the int64 range. Pinning that result would detect changes such as silent overflow or a different exception type.

uros-b
uros-b reviewed Jul 22, 2026
View reviewed changes
Comment thread python/pyspark/tests/upstream/pyarrow/test_pyarrow_arrow_to_pandas_coerce_temporal.py
and empty.
"""

def compare_or_generate_golden_matrix(

@uros-b uros-b Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we move compare_or_generate_golden_matrix into GoldenFileTestMixin? This implementation duplicates the helper in test_pyarrow_arrow_to_pandas_default. Centralizing it alongside the existing golden-file utilities would reduce maintenance.

@Spenserrrr Spenserrrr Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @uros-b, I will add the two test cases. For putting compare_or_generate_golden_matrix into GoldenFileTestMixin, I agreed that we should do so. Also note that it is also duplicated in test_pyarrow_array_cast.py, so I think a proper edit would move it into mixin and update all three. Would you prefer I do that refactor as a separate PR or fold this cleanup here? Happy to do either way.

@Yicong-Huang Yicong-Huang Jul 22, 2026

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.

I second this

@uros-b
uros-b requested a review from Yicong-Huang July 22, 2026 20:43
@uros-b

uros-b commented Jul 22, 2026

Copy link
Copy Markdown
Member

Adding @Yicong-Huang for further PySpark review

@Spenserrrr Spenserrrr changed the title [SPARK-54946][PYTHON][TEST] Add tests for pa.Array.to_pandas with coe… [SPARK-54946][PYTHON][TEST] Add tests for pa.Array.to_pandas with coerce_temporal_nanoseconds Jul 22, 2026
Yicong-Huang
Yicong-Huang reviewed Jul 22, 2026
View reviewed changes

@Yicong-Huang Yicong-Huang 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.

Thanks @Spenserrrr , left comments inline

Comment thread python/pyspark/tests/upstream/pyarrow/test_pyarrow_arrow_to_pandas_coerce_temporal.py
Comment on lines +244 to +248
# Version-specific expected values go here, keyed by (row, col), when a
# newer pandas/PyArrow/NumPy legitimately changes a cell's output.
# add a LooseVersion-guarded block for each known drift.
overrides: dict[tuple[str, str], str] = {}
# Pandas 3 uses its dedicated string dtype for non-empty Arrow string arrays.

@Yicong-Huang Yicong-Huang Jul 22, 2026

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.

we support pyarrow 18+, can we run this against pyarrow 18 ~ 25, and adds the overrides if any version diff is found?

Comment thread python/pyspark/tests/upstream/pyarrow/test_pyarrow_arrow_to_pandas_coerce_temporal.py
and empty.
"""

def compare_or_generate_golden_matrix(

@Yicong-Huang Yicong-Huang Jul 22, 2026

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.

I second this

Comment thread dev/sparktestsupport/modules.py
# unittests for upstream projects
"pyspark.tests.upstream.pyarrow.test_pyarrow_array_cast",
"pyspark.tests.upstream.pyarrow.test_pyarrow_array_type_inference",
"pyspark.tests.upstream.pyarrow.test_pyarrow_arrow_to_pandas_coerce_temporal",

@Yicong-Huang Yicong-Huang Jul 22, 2026

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.

we might be able to combine those flags into one file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Yicong-Huang Yicong-Huang Yicong-Huang left review comments
@uros-b uros-b uros-b left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Spenserrrr @uros-b @Yicong-Huang