Re: [PR] [SPARK-55324][PYTHON] Make convert_numpy support ArrayType [spark]

["HyukjinKwon (via GitHub)" <gi...@apache.org>]

HyukjinKwon commented on code in PR #54143:
URL: https://github.com/apache/spark/pull/54143#discussion_r3449252322


##########
python/pyspark/sql/conversion.py:
##########
@@ -1712,8 +1825,20 @@ def _prefer_convert_numpy(
         )
         if df_for_struct and isinstance(spark_type, StructType):
             return all(isinstance(f.dataType, supported_types) for f in spark_type.fields)
+        elif isinstance(spark_type, supported_types):
+            return True
+        elif isinstance(spark_type, ArrayType):
+            # Recurse into element type. MapType and StructType are not yet supported.
+            element_type = spark_type.elementType
+            if isinstance(element_type, (MapType, StructType)):
+                return False
+            elif isinstance(element_type, ArrayType):
+                return cls._prefer_convert_numpy(element_type, df_for_struct=False)

Review Comment:
   This ArrayType branch (`return cls._prefer_convert_numpy(...)` here, and the `return True` just below for non-nested elements) makes `_prefer_convert_numpy` true for `Array[E]` whenever `E` is not Map/Struct/nested-Array -- **including** element types that are NOT in the `supported_types` tuple above (`StringType`, `DecimalType`, the interval types), which return `False` at the scalar level.
   
   So `toPandas()` now routes those array columns -- notably the very common `Array[String]` -- through `convert_numpy`, whereas before this PR they used `_create_converter_to_pandas` (ArrayType wasn't preferred). The new tests assert `convert_numpy`'s output value (e.g. `Array[String]` -> `[["a", None, "c"], None]`) but not that it matches the old path's output, so a user-visible representation change (object ndarray vs list, or null encoding) is possible.
   
   This is the concrete form of @zhengruifeng's open question above about behavior differences between the two paths. Could you either add cross-path parity tests for `Array[String]`/`Array[Decimal]`, or restrict this element recursion to `supported_types` membership so unconfirmed element types stay on the old path until parity is confirmed?



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org