HyukjinKwon commented on code in PR #57079:
URL: https://github.com/apache/spark/pull/57079#discussion_r3555785692
##########
sql/api/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -8160,7 +8531,7 @@ object functions {
* @param str
* A string to be parsed to time.
* @return
- * A time, or null if the input is malformed.
+ * A time, or null if the input is malformed. Returns a column that
evaluates to a string.
Review Comment:
Same as `to_time`: `try_to_time` returns `TimeType`, not string.
```suggestion
* A time, or null if the input is malformed. Returns a column that
evaluates to a time.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -11914,13 +12204,16 @@ def to_time(str: "ColumnOrName", format:
Optional["ColumnOrName"] = None) -> Col
----------
str : :class:`~pyspark.sql.Column` or column name
string to be parsed to time.
+ A column that evaluates to a string.
format: :class:`~pyspark.sql.Column` or column name, optional
time format pattern to follow.
+ A column that evaluates to a string.
Returns
-------
:class:`~pyspark.sql.Column`
time value as :class:`pyspark.sql.types.TimeType` type.
+ Returns a column that evaluates to a string.
Review Comment:
Wrong return type — mirrors the Scala `to_time`: this line sits directly
under "time value as `pyspark.sql.types.TimeType` type.", so "string"
contradicts it.
```suggestion
Returns a column that evaluates to a time.
```
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/bitwiseExpressions.scala:
##########
@@ -78,6 +85,13 @@ case class BitwiseAnd(left: Expression, right: Expression)
extends BinaryArithme
*/
@ExpressionDescription(
usage = "expr1 _FUNC_ expr2 - Returns the result of bitwise OR of `expr1`
and `expr2`.",
+ arguments = """
+ Arguments:
+ * expr1 - The expr1 argument.
+ An expression that evaluates to an integral.
+ * expr2 - The expr2 argument.
Review Comment:
These arg descriptions just restate the parameter name ("The expr1
argument.") — no information. The sibling `BitwiseAnd`/`BitwiseXor` in this
file use "The first/second operand of the bitwise AND / exclusive OR."; suggest
matching them:
```suggestion
* expr1 - The first operand of the bitwise OR.
An expression that evaluates to an integral.
* expr2 - The second operand of the bitwise OR.
```
##########
python/pyspark/sql/functions/builtin.py:
##########
@@ -12056,13 +12351,16 @@ def try_to_time(str: "ColumnOrName", format:
Optional["ColumnOrName"] = None) ->
----------
str : :class:`~pyspark.sql.Column` or column name
string to be parsed to time.
+ A column that evaluates to a string.
format: :class:`~pyspark.sql.Column` or column name, optional
time format pattern to follow.
+ A column that evaluates to a string.
Returns
-------
:class:`~pyspark.sql.Column`
time value as :class:`pyspark.sql.types.TimeType` type.
+ Returns a column that evaluates to a string.
Review Comment:
Same as `to_time`: `try_to_time` returns `TimeType`, not string.
```suggestion
Returns a column that evaluates to a time.
```
##########
sql/api/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -8093,7 +8461,8 @@ object functions {
* @param str
* A string to be parsed to time.
* @return
- * A time, or raises an error if the input is malformed.
+ * A time, or raises an error if the input is malformed. Returns a column
that evaluates to a
+ * string.
Review Comment:
Wrong return type: `to_time` returns `TimeType` (`ToTime.dataType =
TimeType()`), not string — the prose right here even says "A time". Same on
`try_to_time` (8534) and the PySpark mirrors (builtin.py:12216, 12363). This
has been here since the first reviewed revision, so it's my own late catch, not
from a recent commit.
```suggestion
* A time, or raises an error if the input is malformed. Returns a
column that evaluates to a
* time.
```
--
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]
