Skip to content

[SPARK-58656][PYTHON][TESTS] Centralize the to_pandas golden test inventory into a shared base - #57861

Open
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:chunkedarray-to-pandas-tests
Open

[SPARK-58656][PYTHON][TESTS] Centralize the to_pandas golden test inventory into a shared base#57861
Spenserrrr wants to merge 1 commit into
apache:masterfrom
Spenserrrr:chunkedarray-to-pandas-tests

Conversation

@Spenserrrr

@Spenserrrr Spenserrrr commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This is a pure refactor of the existing pa.Array.to_pandas golden tests, done first so that pa.ChunkedArray.to_pandas coverage can be added cleanly in a follow-up. It adds no new test cases and changes no behavior; it only reorganizes the shared machinery.

  • Move _PyArrowToPandasTestBase into test_pyarrow_arrow_to_pandas_default.py and have PyArrowArrayToPandasDefaultTests extend it, so the base class is the single home for the machinery both test files share.
  • Split the source-array inventory into type-family group methods on the base -- _numeric_sources, _string_binary_sources, _temporal_sources, _nested_sources, _dictionary_sources -- unioned by _build_source_arrays. A concrete test can now reuse the whole inventory or one group.
  • PyArrowArrayToPandasCoerceTemporalTests builds its rows from _temporal_sources() instead of rewriting the temporal types, removing the duplicated definitions. Its golden file is regenerated: same rows and same cell values, only the row order changes to match the shared temporal group.
  • The default test class reuses the base's _to_pandas_cell instead of reimplementing the conversion inline.
  • Move _verify_zero_copy / _arrow_buffers (used only by the zero-copy tests) out of the base and into PyArrowArrayToPandasZeroCopyTests, so no class inherits helpers it does not use.
  • The non-default file now imports the shared base directly rather than importing the whole default module to avoid test re-collection; a base with no test_* methods is not collected.

Class hierarchy after this change:

GoldenFileTestMixin
  <- _PyArrowToPandasTestBase                 (in ..._default.py; _to_pandas_cell + 5 group methods + _build_source_arrays; no test_*)
       <- PyArrowArrayToPandasDefaultTests               (..._default.py)
       <- PyArrowArrayToPandasCoerceTemporalTests        (..._non_default.py; uses _temporal_sources())
       <- PyArrowArrayToPandasZeroCopyTests              (..._non_default.py; owns _verify_zero_copy / _arrow_buffers)
       <- PyArrowArrayToPandasIntegerObjectNullsTests    (..._non_default.py)

Why are the changes needed?

pa.ChunkedArray.to_pandas shares to_pandas's entire keyword surface and returns a Series, exactly like pa.Array.to_pandas, so its tests belong in these same files. Adding them requires the source inventory to be reusable by group (the ChunkedArray rows extend the existing ones) and the shared machinery to live in one place. Doing this reorganization on its own keeps the follow-up focused purely on the new coverage, and keeps this diff easy to verify as behavior-preserving.

I will add the pa.ChunkedArray.to_pandas tests in a follow-up PR after this one is merged.

Does this PR introduce any user-facing change?

No. Test-only refactor.

How was this patch tested?

  • The four to_pandas golden files were regenerated. The default, zero_copy, zero_copy_arrow_backed, and integer_object_nulls goldens are byte-identical; the coerce_temporal golden differs only in row order (same rows, same cell values, confirmed by a sorted diff).
  • The full to_pandas test suite passes without SPARK_GENERATE_GOLDEN_FILES, and passes across a sweep of pyarrow 18/19/20/21/22/23/24/25 x pandas 2 and 3 (16/16).
  • dev/lint-python --ruff is clean.

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

Generated-by: Claude Code (Opus 4.8)

@Spenserrrr
Spenserrrr marked this pull request as ready for review August 7, 2026 20:58
@Spenserrrr

Copy link
Copy Markdown
Contributor Author

Hi @Yicong-Huang @zhengruifeng! This is a PR on monitoring upstream library behavior. Could you take a look when you have time? Thank you!

@uros-b

uros-b commented Aug 8, 2026

Copy link
Copy Markdown
Member

Thank you @Spenserrrr! Adding @Yicong-Huang for further review

@uros-b
uros-b requested a review from Yicong-Huang August 8, 2026 11:02
…entory into a shared base

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

This is a pure refactor of the existing `pa.Array.to_pandas` golden tests, in
preparation for adding `pa.ChunkedArray.to_pandas` coverage in a follow-up. No
new test cases and no behavior change; it only reorganizes the shared machinery
so the ChunkedArray tests can slot in cleanly.

- Move `_PyArrowToPandasTestBase` into `test_pyarrow_arrow_to_pandas_default.py`
  and have `PyArrowArrayToPandasDefaultTests` extend it, so the base is the single
  home for machinery both files share.
- Split the source-array inventory into type-family group methods
  (`_numeric_sources`, `_string_binary_sources`, `_temporal_sources`,
  `_nested_sources`, `_dictionary_sources`) on the base, unioned by
  `_build_source_arrays`. A test can now reuse the whole inventory or one group.
- `PyArrowArrayToPandasCoerceTemporalTests` now builds its rows from
  `_temporal_sources()` instead of hand-rolling the temporal types, removing the
  duplicated definitions. Its golden file is regenerated: same rows and same cell
  values, only the row order changes to match the shared temporal group.
- The default test class now reuses the base's `_to_pandas_cell` instead of
  reimplementing the conversion inline.
- Move `_verify_zero_copy` / `_arrow_buffers` (used only by the zero-copy tests)
  out of the base and into `PyArrowArrayToPandasZeroCopyTests`, so no class
  inherits helpers it does not use.
- The non-default file now imports the shared base directly rather than importing
  the whole default module to dodge test re-collection; importing a base with no
  `test_*` methods does not trigger collection.

### Why are the changes needed?

`pa.ChunkedArray.to_pandas` shares `to_pandas`'s entire keyword surface and Series
result shape, so its tests belong in these same files. Adding them first requires
the source inventory to be reusable by group (the ChunkedArray rows extend the
existing ones) and the shared machinery to live in one place. Doing that
reorganization on its own keeps the follow-up focused on the new coverage.

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

No. Test-only refactor.

### How was this patch tested?

- The four to_pandas golden files were regenerated; the default, zero-copy,
  zero-copy-arrow-backed, and integer-object-nulls goldens are byte-identical, and
  the coerce-temporal golden differs only in row order (same rows, same values).
- The full test suite passes without `SPARK_GENERATE_GOLDEN_FILES`, and passes
  across a sweep of pyarrow 18-25 x pandas 2 and 3 (16/16).
- `dev/lint-python --ruff` clean.

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

Generated-by: Claude Code (Opus 4.8)
@Spenserrrr
Spenserrrr force-pushed the chunkedarray-to-pandas-tests branch from 0763c5b to b02022a Compare August 8, 2026 19:09
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