gaogaotiantian commented on code in PR #57099:
URL: https://github.com/apache/spark/pull/57099#discussion_r3561857051
##########
python/pyspark/sql/conversion.py:
##########
@@ -980,6 +1017,64 @@ class ArrowTableToRowsConversion:
Conversion from Arrow Table to Rows.
"""
+ @staticmethod
+ def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
+ """
+ Equivalent to ``column.to_pylist()``, but converts (nested) list
columns in bulk
+ instead of one scalar at a time.
+
+ ``Array.to_pylist()`` materializes one Scalar per element; for list
types each row
+ additionally allocates a C++ scalar, a Python Scalar wrapper and a
Python Array
+ wrapper for the row's values before converting elements one by one,
which is
+ several times slower than converting the flattened child values in a
single pass
+ and slicing the resulting Python list per row (see
apache/arrow#50326). The values
+ themselves are still converted by Arrow's own ``to_pylist``, so
results are exactly
+ identical: ``None`` stays ``None`` and values inside numeric lists
stay Python ints,
+ unlike a pandas round trip which would coerce them to floats/NaN.
NumPy is used
+ only for the offsets (non-null integers) and the validity bitmap
(booleans), so no
+ value coercion can occur.
+
+ This can be removed once the minimum supported PyArrow version
includes the fix
+ for apache/arrow#50326.
+ """
+ import pyarrow as pa
+ import pyarrow.types as pa_types
Review Comment:
Let's not do this. Just use `pa.types`. We use `pa_type` a lot in this file
as a pyarrow datatype variable. It would be confusing to developers when this
now becomes a module. LLM sometimes overestimate the cost to access an
attribute of an object - 99% of the time this does not make a difference at all.
##########
python/pyspark/sql/conversion.py:
##########
@@ -980,6 +1017,64 @@ class ArrowTableToRowsConversion:
Conversion from Arrow Table to Rows.
"""
+ @staticmethod
+ def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
+ """
+ Equivalent to ``column.to_pylist()``, but converts (nested) list
columns in bulk
+ instead of one scalar at a time.
+
+ ``Array.to_pylist()`` materializes one Scalar per element; for list
types each row
+ additionally allocates a C++ scalar, a Python Scalar wrapper and a
Python Array
+ wrapper for the row's values before converting elements one by one,
which is
+ several times slower than converting the flattened child values in a
single pass
+ and slicing the resulting Python list per row (see
apache/arrow#50326). The values
+ themselves are still converted by Arrow's own ``to_pylist``, so
results are exactly
+ identical: ``None`` stays ``None`` and values inside numeric lists
stay Python ints,
+ unlike a pandas round trip which would coerce them to floats/NaN.
NumPy is used
+ only for the offsets (non-null integers) and the validity bitmap
(booleans), so no
+ value coercion can occur.
+
+ This can be removed once the minimum supported PyArrow version
includes the fix
+ for apache/arrow#50326.
+ """
+ import pyarrow as pa
+ import pyarrow.types as pa_types
+
+ if _has_fast_native_to_pylist() or not _is_numpy_available():
+ # Recent PyArrow converts without per-element Scalars natively
+ # (apache/arrow#50326); without NumPy the bulk paths below are
+ # unavailable. Either way, use the native conversion.
+ return column.to_pylist()
+
+ if isinstance(column, pa.ChunkedArray):
+ result: List[Any] = []
+ for chunk in column.chunks:
+ result.extend(ArrowTableToRowsConversion._to_pylist(chunk))
+ return result
+
+ column_type = column.type
Review Comment:
I don't think this is really necessary.
##########
python/pyspark/sql/conversion.py:
##########
@@ -980,6 +1017,64 @@ class ArrowTableToRowsConversion:
Conversion from Arrow Table to Rows.
"""
+ @staticmethod
+ def _to_pylist(column: Union["pa.Array", "pa.ChunkedArray"]) -> List[Any]:
+ """
+ Equivalent to ``column.to_pylist()``, but converts (nested) list
columns in bulk
+ instead of one scalar at a time.
+
+ ``Array.to_pylist()`` materializes one Scalar per element; for list
types each row
+ additionally allocates a C++ scalar, a Python Scalar wrapper and a
Python Array
+ wrapper for the row's values before converting elements one by one,
which is
+ several times slower than converting the flattened child values in a
single pass
+ and slicing the resulting Python list per row (see
apache/arrow#50326). The values
+ themselves are still converted by Arrow's own ``to_pylist``, so
results are exactly
+ identical: ``None`` stays ``None`` and values inside numeric lists
stay Python ints,
+ unlike a pandas round trip which would coerce them to floats/NaN.
NumPy is used
+ only for the offsets (non-null integers) and the validity bitmap
(booleans), so no
+ value coercion can occur.
+
+ This can be removed once the minimum supported PyArrow version
includes the fix
+ for apache/arrow#50326.
+ """
+ import pyarrow as pa
+ import pyarrow.types as pa_types
+
+ if _has_fast_native_to_pylist() or not _is_numpy_available():
+ # Recent PyArrow converts without per-element Scalars natively
+ # (apache/arrow#50326); without NumPy the bulk paths below are
+ # unavailable. Either way, use the native conversion.
+ return column.to_pylist()
+
+ if isinstance(column, pa.ChunkedArray):
+ result: List[Any] = []
Review Comment:
Is the type hint here necessary? I thought `[]` indicates `List[Any]`. Maybe
I'm wrong. It's possible that due to the closure this requires some annotation.
--
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]
