HyukjinKwon commented on code in PR #59326:
URL: https://github.com/apache/spark/pull/59326#discussion_r4238252067
##########
python/pyspark/sql/conversion.py:
##########
@@ -543,7 +544,9 @@ def _convert_column_legacy(
# trigger the cast fallback when arrow_cast is enabled.
try:
try:
- return pa.Array.from_pandas(series, mask=mask,
type=arrow_type, safe=safecheck)
+ return _create_array_from_pandas(
Review Comment:
**Nit (P3):** No test covers this legacy-mode call with a pyarrow-backed
series. It is the branch `worker.py`'s `convert_df_to_arrow` uses for Arrow
UDTF output. The new SPARK-60009 tests either go through
`create_arrow_array_from_pandas` or call `from_pandas` with the default
`use_legacy_error_handling=False`. `test_from_pandas_use_legacy_error_handling`
only uses inputs that fail under both the old and new code.
If this line went back to `pa.Array.from_pandas(series, mask=mask,
type=arrow_type, safe=safecheck)`, a `pd.ArrowDtype(pa.int32())` series for a
`LongType` field would come back as int32 again on PyArrow 18. The protocol
ignores the type and nothing raises, so the `arrow_cast` fallback never runs,
and no test would fail.
Could you add a `use_legacy_error_handling=True, arrow_cast=True` variant of
`test_from_pandas_arrow_dtype_requested_type`? Like the other requested-type
checks, it would only fail on the PyArrow 18 job, but it would cover this
branch.
**Verification:**
- **Regression:** Legacy-mode conversion of a pyarrow-backed series yields
the requested Arrow type and values, and the check fails on pyarrow < 19 when
that call site bypasses the shared helper.
- **Compatibility:** The existing conversion tests still pass, including the
legacy error-format test and the non-legacy requested-type, safety,
offset-width, and nullable-dtype tests.
##########
python/pyspark/sql/pandas/conversion.py:
##########
@@ -58,14 +58,33 @@
from pyspark.sql.pandas._typing import DataFrameLike as PandasDataFrameLike
+def _create_array_from_pandas(
+ series: "pd.Series",
+ arrow_type: Optional["pa.DataType"],
+ *,
+ mask: Optional["pd.Series"],
+ safecheck: bool,
+) -> Union["pa.Array", "pa.ChunkedArray"]:
+ import pyarrow as pa
+
+ if mask is None:
Review Comment:
Thanks, confirmed. `_create_array_from_pandas` now takes the
extract-then-cast path only for `pd.arrays.ArrowExtensionArray`, so `Int64`,
`Float64`, `boolean`, and `string[python]` keep the typed
`pa.Array.from_pandas(..., type=...)` call and still raise on mismatches.
`test_nullable_dtype_type_mismatch` and `test_nullable_dtype_requested_type`
cover this through both `PandasToArrowConversion.from_pandas` and
`create_arrow_array_from_pandas`, and they pass for me on pyarrow 25.0.1 /
pandas 3.0.6.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4237806937","thread_id":"inline:4237806937","verdict_sha256":"88252b55a6909ad22fa8d910e12351cff39c30e7209785e39c45de82d7b0151b"}
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
