HyukjinKwon commented on code in PR #59326:
URL: https://github.com/apache/spark/pull/59326#discussion_r4237806939
##########
python/pyspark/sql/tests/test_conversion.py:
##########
@@ -439,6 +440,73 @@ def test_from_pandas_arrow_cast(self):
result = PandasToArrowConversion.from_pandas(df, schema,
arrow_cast=True)
self.assertEqual(result.column(0).to_pylist(), [1, 2, 3])
+ def test_from_pandas_arrow_dtype_requested_type(self):
Review Comment:
**Nit (P3):** All five new tests use `pd.ArrowDtype` series, but
`_create_array_from_pandas` takes the same branch for every `__arrow_array__`
input, including `Int64`, `Float64`, `boolean`, and `string[python]`. Nothing
here, in `arrow/test_arrow.py`, or in the coercion goldens converts those
dtypes to a mismatched type or uses `safecheck=False`. As a result, the
nullable-dtype coercion change in the helper passes the suite. Could you add
nullable-dtype cases that pin the intended behavior, through both
`PandasToArrowConversion.from_pandas` and `create_arrow_array_from_pandas`?
See **Shared repair plan 1** in the review body.
##########
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:
**Non-blocking (P2):** Here `mask is None` means "the array has
`__arrow_array__`", which is broader than "the array is Arrow-backed". It is
also true for pandas' NumPy-backed nullable arrays (`Int64`, `Float64`,
`boolean`, ...) and for `string[python]`. Their `__arrow_array__` already
honors the requested type (`pa.array(self._data, mask=self._mask, type=type)`),
so before this change a mismatched type raised `ArrowTypeError`. They now go
through `Array.cast` instead, on every PyArrow version:
- A pandas UDF declared `StringType` that returns `pd.Series([1, None],
dtype="Int64")` used to fail with `PySparkTypeError`. It now returns `['1',
None]`; `Float64` gives `'1.5'` and `boolean` gives `'true'`. A NumPy `int64`
series with the same values still fails. This bypasses the non-legacy rule in
`PandasToArrowConversion._convert_column_legacy` ("ArrowTypeError must NOT be
silently cast").
- `createDataFrame` with a `StringType` schema accepts these columns the
same way, and `string[python]` values are now parsed into integer columns.
- `Int64` for `BinaryType` or `StructType` now escapes as a raw
`ArrowNotImplementedError` instead of a `PySpark*Error` or the previous result.
I confirmed these by running the old and new code paths on pyarrow 25.0.1 /
pandas 3.0.6. Could this path apply only when `isinstance(series.array,
pd.arrays.ArrowExtensionArray)`? That covers `pd.ArrowDtype`,
`string[pyarrow]`, and pandas 3's default `str`. Every other `__arrow_array__`
implementer would keep the typed `pa.Array.from_pandas(series, mask=mask,
type=arrow_type, safe=safecheck)` call.
See **Shared repair plan 1** in the review body.
--
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]
