[GitHub] [spark] zero323 commented on a diff in pull request #37748: [SPARK-40210][PYTHON][CORE] Fix math atan2, hypot, pow and pmod float argument call

2022-09-01 Thread GitBox


zero323 commented on code in PR #37748:
URL: https://github.com/apache/spark/pull/37748#discussion_r960952519


##
python/pyspark/sql/functions.py:
##
@@ -108,13 +108,10 @@ def _invoke_binary_math_function(name: str, col1: Any, 
col2: Any) -> Column:
 Invokes binary JVM math function identified by name
 and wraps the result with :class:`~pyspark.sql.Column`.
 """
-return _invoke_function(
-name,
-# For legacy reasons, the arguments here can be implicitly converted 
into floats,
-# if they are not columns or strings.
-_to_java_column(col1) if isinstance(col1, (str, Column)) else 
float(col1),
-_to_java_column(col2) if isinstance(col2, (str, Column)) else 
float(col2),
-)
+
+# For legacy reasons, the arguments here can be implicitly converted into 
column
+cols = [_to_java_column(c if isinstance(c, (str, Column)) else lit(c)) for 
c in (col1, col2)]

Review Comment:
   JVM literal column can be created directly with 
[`_create_column_from_literal`](https://github.com/apache/spark/blob/fdc11ab0494a681444e7a7e13f3f99d25fa6cf2f/python/pyspark/sql/column.py#L47-L50).
   



-- 
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-unsubscr...@spark.apache.org

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


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



[GitHub] [spark] zero323 commented on a diff in pull request #37748: [SPARK-40210][PYTHON][CORE] Fix math atan2, hypot, pow and pmod float argument call

2022-09-01 Thread GitBox


zero323 commented on code in PR #37748:
URL: https://github.com/apache/spark/pull/37748#discussion_r960525054


##
python/pyspark/sql/functions.py:
##
@@ -108,12 +108,13 @@ def _invoke_binary_math_function(name: str, col1: Any, 
col2: Any) -> Column:
 Invokes binary JVM math function identified by name
 and wraps the result with :class:`~pyspark.sql.Column`.
 """
+
+def align_type(c: Any) -> Union[str, Column]:
+# For legacy reasons, the arguments here can be implicitly converted 
into column
+return c if isinstance(c, (str, Column)) else lit(c)

Review Comment:
   Unless we plan to reuse this in the future, I'd make more senses to just 
inline the expression.
   
   
   If we do plan to reuse it, this should be marked as internal (`align_type` 
-> `_align_type`) and moved next to other helper functions (also, a docstring 
would be a good idea).



-- 
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-unsubscr...@spark.apache.org

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


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