You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by misutoth <gi...@git.apache.org> on 2018/02/15 09:04:40 UTC
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
GitHub user misutoth opened a pull request:
https://github.com/apache/spark/pull/20618
[SPARK-23329][SQL] Fix documentation of trigonometric functions
## What changes were proposed in this pull request?
Provide more details in trigonometric function documentations. Referenced `java.lang.Math` for further details in the descriptions.
## How was this patch tested?
Ran full build, checked generated documentation manually
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/misutoth/spark trigonometric-doc
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20618.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20618
----
commit 25c329b4f93b407f53d87e8199444e83b6a1be15
Author: Mihaly Toth <mt...@...>
Date: 2018-02-13T14:34:26Z
[SPARK-23329][SQL] Fix documentation of trigonometric functions
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87860 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87860/testReport)** for PR 20618 at commit [`10afda5`](https://github.com/apache/spark/commit/10afda5d97146af7e02cc3b18968f89f074e093c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169086344
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,168 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @return angle in radians whose cosine is `e`, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
+ // scalastyle:off line.size.limit
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @return angle in radians whose cosine is `columnName` as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(columnName: String): Column = acos(Column(columnName))
/**
- * Computes the sine inverse of the given value; the returned angle is in the range
- * -pi/2 through pi/2.
+ * @return angle in radians whose sine is `e`, as if computed by [[java.lang.Math#asin]]
*
* @group math_funcs
* @since 1.4.0
*/
def asin(e: Column): Column = withExpr { Asin(e.expr) }
/**
- * Computes the sine inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2.
+ * @return angle in radians whose sine is `columnName`, as if computed by [[java.lang.Math#asin]]
*
* @group math_funcs
* @since 1.4.0
*/
def asin(columnName: String): Column = asin(Column(columnName))
/**
- * Computes the tangent inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2
+ * @return angle in radians whose tangent is `e`, as if computed by [[java.lang.Math#atan]]
*
* @group math_funcs
* @since 1.4.0
*/
def atan(e: Column): Column = withExpr { Atan(e.expr) }
+ // scalastyle:off line.size.limit
/**
- * Computes the tangent inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2
+ * @return angle in radians whose tangent is `columnName`, as if computed by [[java.lang.Math#atan]]
*
* @group math_funcs
* @since 1.4.0
*/
def atan(columnName: String): Column = atan(Column(columnName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta). Units in radians.
+ *
--- End diff --
Tiny nit: remove this first blank line.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87798 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87798/testReport)** for PR 20618 at commit [`fcf1338`](https://github.com/apache/spark/commit/fcf1338a3345c3f57f274f0e9bea813a24fbe581).
* This patch **fails Python style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168510272
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -770,7 +837,14 @@ case class Unhex(child: Expression) extends UnaryExpression with ImplicitCastInp
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr1, expr2) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`expr1`, `expr2`).",
--- End diff --
It looks like the declaration of atan2 should group with the trig functions. I think that's OK to fix here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20618
Was the result to add more changes to this PR or add them in another PR?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20618
Also cc @srinathshankar
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87796 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87796/testReport)** for PR 20618 at commit [`0e4e15a`](https://github.com/apache/spark/commit/0e4e15a178e7484b2d3706adc8994f226ce0f29a).
* This patch **fails Python style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87796/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169187041
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -804,7 +858,6 @@ case class Pow(left: Expression, right: Expression)
}
}
-
--- End diff --
Sure
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87914 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87914/testReport)** for PR 20618 at commit [`627e204`](https://github.com/apache/spark/commit/627e204ed03cfd6caa06e8f64dc605b62f4d2e5e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169312315
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +272,10 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
+ arguments = """
+ Arguments:
+ * expr - hyperbolic angle.
--- End diff --
ok
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171829946
--- Diff: python/pyspark/sql/functions.py ---
@@ -173,16 +172,26 @@ def _():
_functions_2_1 = {
# unary math functions
- 'degrees': 'Converts an angle measured in radians to an approximately equivalent angle ' +
- 'measured in degrees.',
- 'radians': 'Converts an angle measured in degrees to an approximately equivalent angle ' +
- 'measured in radians.',
+ 'degrees': """Converts an angle measured in radians to an approximately equivalent angle
+ measured in degrees.
+ :param col: angle in radians
+ :return: angle in degrees, as if computed by `java.lang.Math.toDegrees()`""",
+ 'radians': """Converts an angle measured in degrees to an approximately equivalent angle measured in radians.
--- End diff --
Seems this hits the line length limit.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169086090
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,168 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @return angle in radians whose cosine is `e`, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
+ // scalastyle:off line.size.limit
--- End diff --
You should be able to just wrap comments rather than disable this?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168526385
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,178 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @param e the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
+ * as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @param colName the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
+ * as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
- def acos(columnName: String): Column = acos(Column(columnName))
+ def acos(colName: String): Column = acos(Column(colName))
--- End diff --
columnName was too long and it ran into the description in the generated doc.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168525504
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,178 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @param e the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
--- End diff --
Ok.
Then I will cut the part about the domains and ranges.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168578156
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,178 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @param e the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
--- End diff --
I am not sure what you mean on _above_. Do you mean reverting this part of the change?
How about simply `@return the angle whose cosine is 'e'` and refer to java.lang.Math for further details?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168504378
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +285,11 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
+ arguments =
+ """
+ Arguments:
+ * expr - number whose hyperbolic consine is to be returned.
--- End diff --
consine -> cosine
For the hyperbolic functions, you can just describe the argument as a "hyperbolic angle". This description is redundant with the "Returns ..." description above.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87883 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87883/testReport)** for PR 20618 at commit [`10afda5`](https://github.com/apache/spark/commit/10afda5d97146af7e02cc3b18968f89f074e093c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169311232
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2005,71 +1967,63 @@ object functions {
def signum(columnName: String): Column = signum(Column(columnName))
/**
- * @param e angle in radians
- * @return sine of the angle, as if computed by [[java.lang.Math#sin]]
+ * Computes the sine of the given value. Units in radians.
*
* @group math_funcs
* @since 1.4.0
*/
def sin(e: Column): Column = withExpr { Sin(e.expr) }
/**
- * @param columnName angle in radians
- * @return sine of the angle, as if computed by [[java.lang.Math#sin]]
+ * Computes the sine of the given column.
*
* @group math_funcs
* @since 1.4.0
*/
def sin(columnName: String): Column = sin(Column(columnName))
/**
- * @param e hyperbolic angle
- * @return hyperbolic sine of the given value, as if computed by [[java.lang.Math#sinh]]
+ * Computes the hyperbolic sine of the given value.
*
* @group math_funcs
* @since 1.4.0
*/
def sinh(e: Column): Column = withExpr { Sinh(e.expr) }
/**
- * @param columnName hyperbolic angle
- * @return hyperbolic sine of the given value, as if computed by [[java.lang.Math#sinh]]
+ * Computes the hyperbolic sine of the given column.
*
* @group math_funcs
* @since 1.4.0
*/
def sinh(columnName: String): Column = sinh(Column(columnName))
/**
- * @param e angle in radians
- * @return tangent of the given value, as if computed by [[java.lang.Math#tan]]
+ * Computes the tangent of the given value. Units in radians.
*
* @group math_funcs
* @since 1.4.0
*/
def tan(e: Column): Column = withExpr { Tan(e.expr) }
/**
- * @param columnName angle in radians
- * @return tangent of the given value, as if computed by [[java.lang.Math#tan]]
+ * Computes the tangent of the given column.
*
* @group math_funcs
* @since 1.4.0
*/
def tan(columnName: String): Column = tan(Column(columnName))
/**
- * @param e hyperbolic angle
- * @return hyperbolic tangent of the given value, as if computed by [[java.lang.Math#tanh]]
--- End diff --
This way the description and the return value text would be redundant. For this reason it was proposed [during the discussion of the issue](https://issues.apache.org/jira/browse/SPARK-23329?focusedCommentId=16354178&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16354178) to keep only the `@return` part.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168511172
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,178 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @param e the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
--- End diff --
I'd use the same description here as above, for all of these functions. The one above looks better to me. Up to you whether you want to talk about the range of the return value or leave that to the Math.acos docs; it should just be consistent.
Also "cosine inverse" -> "inverse cosine", and make it clear as you do above that it's a synonym for arc cosine.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168511869
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2873,7 +2945,7 @@ object functions {
* or equal to the `windowDuration`. Check
* `org.apache.spark.unsafe.types.CalendarInterval` for valid duration
* identifiers. This duration is likewise absolute, and does not vary
- * according to a calendar.
+ * according to a calendar.
--- End diff --
This should be reverted.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/20618
@misutoth what exactly is the problem you are running into?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
I included java.lang.Math references in functions.R
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87796 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87796/testReport)** for PR 20618 at commit [`0e4e15a`](https://github.com/apache/spark/commit/0e4e15a178e7484b2d3706adc8994f226ce0f29a).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
I just thought about this again and yup I am fine. @misutoth shall we just fix R and Python ones too here? I think we could just target to fix the current functions in other language's API.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168511361
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,178 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @param e the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
+ * as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @param colName the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
+ * as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
- def acos(columnName: String): Column = acos(Column(columnName))
+ def acos(colName: String): Column = acos(Column(colName))
--- End diff --
Why change columnName -> colName? doesn't seem to matter, so I'd avoid changing it if so.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171167374
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -512,7 +529,11 @@ case class Rint(child: Expression) extends UnaryMathExpression(math.rint, "ROUND
case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "SIGNUM")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the sine of `expr`.",
+ usage = "_FUNC_(expr) - Returns the sine of `expr`, as if computed by `java.lang.Math._FUNC_`.",
--- End diff --
I don't know the history of why the text is different here vs python/R https://github.com/apache/spark/blob/master/R/pkg/R/functions.R#L1466
I don't think the doc grouping in R is relevant - the text is there. Generally we try to match the text in scala but I don't feel strongly either way in this case.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171834617
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1500,31 +1534,35 @@ object functions {
}
/**
- * Computes the cosine of the given value. Units in radians.
+ * @param e angle in radians
+ * @return cosine of the angle, as if computed by [[java.lang.Math#cos]]
--- End diff --
Seems this link is broken in Scaladoc. Let's just use `` `java.lang.Math.cos` `` in this file.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169213138
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +273,11 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
+ arguments =
+ """
--- End diff --
I mean ...
```
arguments = """
Arguments:
* expr - hyperbolic angle.
""",
```
to be consistent other styles here. For example:
https://github.com/apache/spark/blob/5683984520cfe9e9acf49e47a84a56af155a8ad2/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/regexpExpressions.scala#L156-L168
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168510579
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -972,6 +1045,7 @@ case class Logarithm(left: Expression, right: Expression)
}
}
+
--- End diff --
I'd revert the whitespace changes here, or at least only make changes that make the spacing consistent.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87860 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87860/testReport)** for PR 20618 at commit [`10afda5`](https://github.com/apache/spark/commit/10afda5d97146af7e02cc3b18968f89f074e093c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169287270
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +273,11 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
+ arguments =
+ """
--- End diff --
You are right, I will fix this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171991691
--- Diff: python/pyspark/sql/functions.py ---
@@ -173,16 +172,26 @@ def _():
_functions_2_1 = {
# unary math functions
- 'degrees': 'Converts an angle measured in radians to an approximately equivalent angle ' +
- 'measured in degrees.',
- 'radians': 'Converts an angle measured in degrees to an approximately equivalent angle ' +
- 'measured in radians.',
+ 'degrees': """Converts an angle measured in radians to an approximately equivalent angle
+ measured in degrees.
--- End diff --
added
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168531352
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -770,7 +837,14 @@ case class Unhex(child: Expression) extends UnaryExpression with ImplicitCastInp
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr1, expr2) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`expr1`, `expr2`).",
+ usage = "_FUNC_(exprY, exprX) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`exprX`, `exprY`), " +
+ "as if computed by `java.lang.Math._FUNC_`.",
+ arguments =
+ """
+ Arguments:
+ * exprY - the ordinate coordinate
+ * exprX - the abscissa coordinate
--- End diff --
Sure, I will fix that. I borrowed that from java.lang.Math ... which sometimes is more complicated than it should be.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87546 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87546/testReport)** for PR 20618 at commit [`168d7b3`](https://github.com/apache/spark/commit/168d7b3715c8ad19b68c9542ffdb90bcb126c312).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182396
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -538,8 +559,14 @@ case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH"
""")
case class Sqrt(child: Expression) extends UnaryMathExpression(math.sqrt, "SQRT")
+// scalastyle:off line.size.limit
--- End diff --
Ditto for turning it on
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169085966
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -770,7 +837,14 @@ case class Unhex(child: Expression) extends UnaryExpression with ImplicitCastInp
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr1, expr2) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`expr1`, `expr2`).",
--- End diff --
I believe this should still be moved with this change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
cc @felixcheung (I saw you and Felix in dev mailing list). So, https://github.com/apache/spark/tree/master/R#generating-documentation does not work?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
@felixcheung, I have started a mail thread on dev@spark.apache.org with title _Help needed in R documentation generation_ because I did not feel it is directly related to this PR. Thanks for your reply on this thread already.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87554/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20618
@HyukjinKwon I think it's fine to refer to `java.lang.Math` even from Python, R documents. It's just expressing the API's behavior, and that contract is the same regardless of the language. (Or is there a subtlety I'm missing, like, it's computed different in Python or the behavior of things like NaN isn't the same?) (Or is your point just that you can't easily write hyperlinks to the JDK docs in Python? that's fine, no need for a link.)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87546/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
Thanks @srowen and @HyukjinKwon for your comments so far ...
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
Maybe I am too much caring about this and It guess it wouldn't be a big change; however, I was thinking there might be subtle nits, like, referring `java.lang.Math*` in Python side and R side which is not preferred up to my knowledge.
Just wanted to avoid another iteration for finding a better description for both Python and R again here. (Just noticed it's his very first contribution to Spark too.).
Seems fine for SQL / Scala / Java as they already use a fully qualified class name, for example, in case of SQL:
```sql
spark-sql> DESCRIBE FUNCTION EXTENDED cos;
...
Function: cos
Class: org.apache.spark.sql.catalyst.expressions.Cos
...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182568
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -804,7 +858,6 @@ case class Pow(left: Expression, right: Expression)
}
}
-
--- End diff --
Seems unrelated change. Could we just revert this back?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168670792
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -521,7 +554,13 @@ case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "S
case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the hyperbolic sine of `expr`.",
+ usage = "_FUNC_(expr) - Returns hyperbolic sine of `expr`, " +
--- End diff --
I think we can just do as below:
```scala
...
usage = """
_FUNC_(expr) - Returns hyperbolic sine of `expr`, as if computed by
`java.lang.Math._FUNC_`.
""",
...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169305100
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +272,10 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
--- End diff --
Shall we add `java.lang.Math. _FUNC_ ` here too?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87551 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87551/testReport)** for PR 20618 at commit [`8550a27`](https://github.com/apache/spark/commit/8550a27636a508a4953076691a5b5a36c7e24824).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87914 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87914/testReport)** for PR 20618 at commit [`627e204`](https://github.com/apache/spark/commit/627e204ed03cfd6caa06e8f64dc605b62f4d2e5e).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20618
Consistency is good. You're suggesting updating Python, R separately? If it's a big change, I could see breaking it up, but could also imagine adjusting that here. I am OK with getting this in first as I know we've iterated on this a lot.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87914/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169299115
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -2005,71 +1967,63 @@ object functions {
def signum(columnName: String): Column = signum(Column(columnName))
/**
- * @param e angle in radians
- * @return sine of the angle, as if computed by [[java.lang.Math#sin]]
+ * Computes the sine of the given value. Units in radians.
*
* @group math_funcs
* @since 1.4.0
*/
def sin(e: Column): Column = withExpr { Sin(e.expr) }
/**
- * @param columnName angle in radians
- * @return sine of the angle, as if computed by [[java.lang.Math#sin]]
+ * Computes the sine of the given column.
*
* @group math_funcs
* @since 1.4.0
*/
def sin(columnName: String): Column = sin(Column(columnName))
/**
- * @param e hyperbolic angle
- * @return hyperbolic sine of the given value, as if computed by [[java.lang.Math#sinh]]
+ * Computes the hyperbolic sine of the given value.
*
* @group math_funcs
* @since 1.4.0
*/
def sinh(e: Column): Column = withExpr { Sinh(e.expr) }
/**
- * @param columnName hyperbolic angle
- * @return hyperbolic sine of the given value, as if computed by [[java.lang.Math#sinh]]
+ * Computes the hyperbolic sine of the given column.
*
* @group math_funcs
* @since 1.4.0
*/
def sinh(columnName: String): Column = sinh(Column(columnName))
/**
- * @param e angle in radians
- * @return tangent of the given value, as if computed by [[java.lang.Math#tan]]
+ * Computes the tangent of the given value. Units in radians.
*
* @group math_funcs
* @since 1.4.0
*/
def tan(e: Column): Column = withExpr { Tan(e.expr) }
/**
- * @param columnName angle in radians
- * @return tangent of the given value, as if computed by [[java.lang.Math#tan]]
+ * Computes the tangent of the given column.
*
* @group math_funcs
* @since 1.4.0
*/
def tan(columnName: String): Column = tan(Column(columnName))
/**
- * @param e hyperbolic angle
- * @return hyperbolic tangent of the given value, as if computed by [[java.lang.Math#tanh]]
--- End diff --
Oh, but the `@param` and `@return` were fine .. I thought a doc like:
```
* Computes the hyperbolic tangent of the given value.
* @param e hyperbolic angle
* @return hyperbolic tangent of the given value, as if computed by [[java.lang.Math#tanh]]
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169086471
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1500,31 +1537,35 @@ object functions {
}
/**
- * Computes the cosine of the given value. Units in radians.
+ * @param e angle in radians
+ * @return cosine of the angle, as if computed by [[java.lang.Math#cos]]
*
* @group math_funcs
* @since 1.4.0
*/
def cos(e: Column): Column = withExpr { Cos(e.expr) }
/**
- * Computes the cosine of the given column.
+ * @param columnName angle in radians
+ * @return cosine of the angle, as if computed by [[java.lang.Math#cos]]
--- End diff --
You don't need to line these up, especially if it makes it extend past the line limit or wrap.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169305293
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -539,7 +563,14 @@ case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH"
case class Sqrt(child: Expression) extends UnaryMathExpression(math.sqrt, "SQRT")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the tangent of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns the tangent of `expr`, as if computed by
+ `java.lang.Math._FUNC_`.
--- End diff --
Seems the two lines above could fix in one line.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171992166
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,165 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @return inverse cosine of `e` in radians, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @return inverse cosine of `columnName`, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(columnName: String): Column = acos(Column(columnName))
/**
- * Computes the sine inverse of the given value; the returned angle is in the range
- * -pi/2 through pi/2.
+ * @return inverse sine of `e` in radians, as if computed by [[java.lang.Math#asin]]
*
* @group math_funcs
* @since 1.4.0
*/
def asin(e: Column): Column = withExpr { Asin(e.expr) }
/**
- * Computes the sine inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2.
+ * @return inverse sine of `columnName`, as if computed by [[java.lang.Math#asin]]
*
* @group math_funcs
* @since 1.4.0
*/
def asin(columnName: String): Column = asin(Column(columnName))
/**
- * Computes the tangent inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2
+ * @return inverse tangent of `e`, as if computed by [[java.lang.Math#atan]]
*
* @group math_funcs
* @since 1.4.0
*/
def atan(e: Column): Column = withExpr { Atan(e.expr) }
/**
- * Computes the tangent inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2
+ * @return inverse tangent of `columnName`, as if computed by [[java.lang.Math#atan]]
*
* @group math_funcs
* @since 1.4.0
*/
def atan(columnName: String): Column = atan(Column(columnName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta). Units in radians.
+ * @param y coordinate on y-axis
+ * @param x coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Column, r: Column): Column = withExpr { Atan2(l.expr, r.expr) }
+ def atan2(y: Column, x: Column): Column = withExpr { Atan2(y.expr, x.expr) }
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param y coordinate on y-axis
+ * @param xName coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Column, rightName: String): Column = atan2(l, Column(rightName))
+ def atan2(y: Column, xName: String): Column = atan2(y, Column(xName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yName coordinate on y-axis
+ * @param x coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(leftName: String, r: Column): Column = atan2(Column(leftName), r)
+ def atan2(yName: String, x: Column): Column = atan2(Column(yName), x)
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yName coordinate on y-axis
+ * @param xName coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(leftName: String, rightName: String): Column =
- atan2(Column(leftName), Column(rightName))
+ def atan2(yName: String, xName: String): Column =
+ atan2(Column(yName), Column(xName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param y coordinate on y-axis
+ * @param xValue coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Column, r: Double): Column = atan2(l, lit(r))
+ def atan2(y: Column, xValue: Double): Column = atan2(y, lit(xValue))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yName coordinate on y-axis
+ * @param xValue coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(leftName: String, r: Double): Column = atan2(Column(leftName), r)
+ def atan2(yName: String, xValue: Double): Column = atan2(Column(yName), xValue)
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yValue coordinate on y-axis
+ * @param x coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Double, r: Column): Column = atan2(lit(l), r)
+ def atan2(yValue: Double, x: Column): Column = atan2(lit(yValue), x)
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yValue coordinate on y-axis
+ * @param xName coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
--- End diff --
Ok. I have removed the non breaking spaces.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87551/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87883/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87547/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87547 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87547/testReport)** for PR 20618 at commit [`2586b0f`](https://github.com/apache/spark/commit/2586b0f25ae0e563374c03d1db37d08aaf0c8c08).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168511774
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -770,7 +837,14 @@ case class Unhex(child: Expression) extends UnaryExpression with ImplicitCastInp
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr1, expr2) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`expr1`, `expr2`).",
+ usage = "_FUNC_(exprY, exprX) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`exprX`, `exprY`), " +
+ "as if computed by `java.lang.Math._FUNC_`.",
+ arguments =
+ """
+ Arguments:
+ * exprY - the ordinate coordinate
+ * exprX - the abscissa coordinate
--- End diff --
Here and below -- it's not clear how ordinate and abscissa relate to the function's description. Above it's described more simply as the coordinates of a point in the plane, and I think that's more recognizable than these terms.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182053
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -512,16 +522,27 @@ case class Rint(child: Expression) extends UnaryMathExpression(math.rint, "ROUND
case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "SIGNUM")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the sine of `expr`.",
+ usage = "_FUNC_(expr) - Returns the sine of `expr`, as if computed by `java.lang.Math._FUNC_`.",
+ arguments =
+ """
+ Arguments:
+ * expr - angle in radians
+ """,
examples = """
Examples:
> SELECT _FUNC_(0);
0.0
""")
case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN")
+// scalastyle:off line.size.limit
--- End diff --
I think we should turn this on again
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171829796
--- Diff: python/pyspark/sql/functions.py ---
@@ -173,16 +172,26 @@ def _():
_functions_2_1 = {
# unary math functions
- 'degrees': 'Converts an angle measured in radians to an approximately equivalent angle ' +
- 'measured in degrees.',
- 'radians': 'Converts an angle measured in degrees to an approximately equivalent angle ' +
- 'measured in radians.',
+ 'degrees': """Converts an angle measured in radians to an approximately equivalent angle
+ measured in degrees.
--- End diff --
Let's add a newline here. Seems doc could be broken.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
Merged to master and branch-2.3.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20618
@misutoth, I am sorry. I missed https://github.com/apache/spark/pull/20618#discussion_r169291756 and discussion in the JIRA.
@srowen, I was thinking about having the same description part in other languages for now. For example,
```
* Computes the hyperbolic tangent of the given value.
```
->
```
* Computes the hyperbolic tangent of the given value.
*
* @param e hyperbolic angle
* @return hyperbolic tangent computed by [[java.lang.Math#tanh]]
```
What do you think about not touching `functions.py` and `functions.R` with the format above even though it might duplicate a bit for now?
My only concern is to have different descriptions in other languages APIs and I am a little bit worried of when we come to fix Python's and R's.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171836519
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,165 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @return inverse cosine of `e` in radians, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @return inverse cosine of `columnName`, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(columnName: String): Column = acos(Column(columnName))
/**
- * Computes the sine inverse of the given value; the returned angle is in the range
- * -pi/2 through pi/2.
+ * @return inverse sine of `e` in radians, as if computed by [[java.lang.Math#asin]]
*
* @group math_funcs
* @since 1.4.0
*/
def asin(e: Column): Column = withExpr { Asin(e.expr) }
/**
- * Computes the sine inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2.
+ * @return inverse sine of `columnName`, as if computed by [[java.lang.Math#asin]]
*
* @group math_funcs
* @since 1.4.0
*/
def asin(columnName: String): Column = asin(Column(columnName))
/**
- * Computes the tangent inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2
+ * @return inverse tangent of `e`, as if computed by [[java.lang.Math#atan]]
*
* @group math_funcs
* @since 1.4.0
*/
def atan(e: Column): Column = withExpr { Atan(e.expr) }
/**
- * Computes the tangent inverse of the given column; the returned angle is in the range
- * -pi/2 through pi/2
+ * @return inverse tangent of `columnName`, as if computed by [[java.lang.Math#atan]]
*
* @group math_funcs
* @since 1.4.0
*/
def atan(columnName: String): Column = atan(Column(columnName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta). Units in radians.
+ * @param y coordinate on y-axis
+ * @param x coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Column, r: Column): Column = withExpr { Atan2(l.expr, r.expr) }
+ def atan2(y: Column, x: Column): Column = withExpr { Atan2(y.expr, x.expr) }
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param y coordinate on y-axis
+ * @param xName coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Column, rightName: String): Column = atan2(l, Column(rightName))
+ def atan2(y: Column, xName: String): Column = atan2(y, Column(xName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yName coordinate on y-axis
+ * @param x coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(leftName: String, r: Column): Column = atan2(Column(leftName), r)
+ def atan2(yName: String, x: Column): Column = atan2(Column(yName), x)
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yName coordinate on y-axis
+ * @param xName coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(leftName: String, rightName: String): Column =
- atan2(Column(leftName), Column(rightName))
+ def atan2(yName: String, xName: String): Column =
+ atan2(Column(yName), Column(xName))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param y coordinate on y-axis
+ * @param xValue coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Column, r: Double): Column = atan2(l, lit(r))
+ def atan2(y: Column, xValue: Double): Column = atan2(y, lit(xValue))
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yName coordinate on y-axis
+ * @param xValue coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(leftName: String, r: Double): Column = atan2(Column(leftName), r)
+ def atan2(yName: String, xValue: Double): Column = atan2(Column(yName), xValue)
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yValue coordinate on y-axis
+ * @param x coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
+ * in polar coordinates that corresponds to the point
+ * (<i>x</i>, <i>y</i>) in Cartesian coordinates,
+ * as if computed by [[java.lang.Math#atan2]]
*
* @group math_funcs
* @since 1.4.0
*/
- def atan2(l: Double, r: Column): Column = atan2(lit(l), r)
+ def atan2(yValue: Double, x: Column): Column = atan2(lit(yValue), x)
/**
- * Returns the angle theta from the conversion of rectangular coordinates (x, y) to
- * polar coordinates (r, theta).
+ * @param yValue coordinate on y-axis
+ * @param xName coordinate on x-axis
+ * @return the <i>theta</i> component of the point
+ * (<i>r</i>, <i>theta</i>)
--- End diff --
nit ` ` -> ` `. Seems broken in Javadoc.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169086289
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,168 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @return angle in radians whose cosine is `e`, as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
+ // scalastyle:off line.size.limit
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @return angle in radians whose cosine is `columnName` as if computed by [[java.lang.Math#acos]]
--- End diff --
It's not a big deal, but this change also makes the way this inverse function is described inconsistent with how it's described above. I think it's best to make them the same.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87803/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20618
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169100931
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -770,7 +837,14 @@ case class Unhex(child: Expression) extends UnaryExpression with ImplicitCastInp
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr1, expr2) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`expr1`, `expr2`).",
--- End diff --
Now I got it. Sorry, earlier I just misunderstood it. Are you proposing to move this case class inside the file to be next to the other trigonometric function? Based on the comments it seems the functions are group based on the number of arguments. This function has 2 arguments that is why it is under `// Binary math functions` section. Even though this does not seem to create big cohesion inside such a group I guess we do not want to reorganize the file now?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168669517
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,178 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
+ * @param e the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
+ * as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
def acos(e: Column): Column = withExpr { Acos(e.expr) }
/**
- * Computes the cosine inverse of the given column; the returned angle is in the range
- * 0.0 through pi.
+ * @param colName the value whose arc cosine is to be returned
+ * @return cosine inverse of the given value in the range of 0.0 through pi,
+ * as if computed by [[java.lang.Math#acos]]
*
* @group math_funcs
* @since 1.4.0
*/
- def acos(columnName: String): Column = acos(Column(columnName))
+ def acos(colName: String): Column = acos(Column(colName))
--- End diff --
I don't think we should change the name for that reason ..
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87554 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87554/testReport)** for PR 20618 at commit [`40da998`](https://github.com/apache/spark/commit/40da9982b0d96a661f7dab4126c619e56a600f94).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169291756
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,165 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
--- End diff --
Seems like a valid point and I tend to agree with the conclusion to create a separate Jira for this. So far the discussion was about {{functions.scala}} but other languages should offer similar descriptions for the same thing. @srowen, can you please comment on this? I will take {{functions.scala}} out for now.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87798/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87803 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87803/testReport)** for PR 20618 at commit [`2ea1f18`](https://github.com/apache/spark/commit/2ea1f18b6c9b5208485c6a007ce4a6366423fbee).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169186651
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -512,16 +522,27 @@ case class Rint(child: Expression) extends UnaryMathExpression(math.rint, "ROUND
case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "SIGNUM")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the sine of `expr`.",
+ usage = "_FUNC_(expr) - Returns the sine of `expr`, as if computed by `java.lang.Math._FUNC_`.",
+ arguments =
+ """
--- End diff --
Yes, I applied to all places in scope.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87547 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87547/testReport)** for PR 20618 at commit [`2586b0f`](https://github.com/apache/spark/commit/2586b0f25ae0e563374c03d1db37d08aaf0c8c08).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169311993
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -252,7 +255,14 @@ case class Ceil(child: Expression) extends UnaryMathExpression(math.ceil, "CEIL"
}
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the cosine of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns the cosine of `expr`, as if computed by
+ `java.lang.Math._FUNC_`.
+ """,
+ arguments = """
+ Arguments:
+ * expr - angle in radians
+ """,
--- End diff --
yes, indeed that is used elsewhere
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168670009
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -196,7 +208,13 @@ case class Asin(child: Expression) extends UnaryMathExpression(math.asin, "ASIN"
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the inverse tangent (a.k.a. arctangent).",
+ usage = "_FUNC_(expr) - Returns the inverse tangent (a.k.a. arc tangent) of `expr`, " +
+ "as if computed by `java.lang.Math._FUNC_`.",
+ arguments =
--- End diff --
Could we just save one line and stick to the same indentation?
```scala
arguments = """
Arguments:
* expr - number whose arc tangent is to be returned.
""",
examples = """
...
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182499
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -562,7 +595,12 @@ case class Cot(child: Expression)
}
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the hyperbolic tangent of `expr`.",
+ usage = "_FUNC_(expr) - Returns the hyperbolic tangent of `expr`, as if computed by `java.lang.Math._FUNC_`.",
--- End diff --
Seems line limit is reached here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169287876
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -521,7 +542,15 @@ case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "S
case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the hyperbolic sine of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns hyperbolic sine of `expr`, as if computed by
--- End diff --
correct
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171991788
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1500,31 +1534,35 @@ object functions {
}
/**
- * Computes the cosine of the given value. Units in radians.
+ * @param e angle in radians
+ * @return cosine of the angle, as if computed by [[java.lang.Math#cos]]
--- End diff --
Replaced all of them as suggested.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169120482
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -770,7 +837,14 @@ case class Unhex(child: Expression) extends UnaryExpression with ImplicitCastInp
// scalastyle:off line.size.limit
@ExpressionDescription(
- usage = "_FUNC_(expr1, expr2) - Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates (`expr1`, `expr2`).",
--- End diff --
Oh, I get it now, that's my mistake. "Binary" means "two arg" and not perhaps "bitwise" or something. Leave it then.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87554 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87554/testReport)** for PR 20618 at commit [`40da998`](https://github.com/apache/spark/commit/40da9982b0d96a661f7dab4126c619e56a600f94).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169305384
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -548,7 +579,14 @@ case class Sqrt(child: Expression) extends UnaryMathExpression(math.sqrt, "SQRT"
case class Tan(child: Expression) extends UnaryMathExpression(math.tan, "TAN")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the cotangent of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns the cotangent of `expr`, as if computed by
+ `1/java.lang.Math._FUNC_`.
--- End diff --
ditto for one line
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169213260
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -521,7 +542,15 @@ case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "S
case class Sin(child: Expression) extends UnaryMathExpression(math.sin, "SIN")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the hyperbolic sine of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns hyperbolic sine of `expr`, as if computed by
--- End diff --
I think we need double spaces ahead - I think the key is consistency but less invasive change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
Looking back I guess we can expect a couple of comments on R and Python side too, though I will target a lower number of them. :) So I am a little bit favoring moving functions.* update into a separate ticket.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87803 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87803/testReport)** for PR 20618 at commit [`2ea1f18`](https://github.com/apache/spark/commit/2ea1f18b6c9b5208485c6a007ce4a6366423fbee).
* This patch **fails Python style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
Sure, lets do that, no problem.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87551 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87551/testReport)** for PR 20618 at commit [`8550a27`](https://github.com/apache/spark/commit/8550a27636a508a4953076691a5b5a36c7e24824).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171991733
--- Diff: python/pyspark/sql/functions.py ---
@@ -173,16 +172,26 @@ def _():
_functions_2_1 = {
# unary math functions
- 'degrees': 'Converts an angle measured in radians to an approximately equivalent angle ' +
- 'measured in degrees.',
- 'radians': 'Converts an angle measured in degrees to an approximately equivalent angle ' +
- 'measured in radians.',
+ 'degrees': """Converts an angle measured in radians to an approximately equivalent angle
+ measured in degrees.
+ :param col: angle in radians
+ :return: angle in degrees, as if computed by `java.lang.Math.toDegrees()`""",
+ 'radians': """Converts an angle measured in degrees to an approximately equivalent angle measured in radians.
--- End diff --
Yes. I wrapped the text.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r171991647
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -512,7 +529,11 @@ case class Rint(child: Expression) extends UnaryMathExpression(math.rint, "ROUND
case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "SIGNUM")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the sine of `expr`.",
+ usage = "_FUNC_(expr) - Returns the sine of `expr`, as if computed by `java.lang.Math._FUNC_`.",
--- End diff --
As far as the trigonometric functions concerned they should be now in sync I think.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169313329
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -548,7 +579,14 @@ case class Sqrt(child: Expression) extends UnaryMathExpression(math.sqrt, "SQRT"
case class Tan(child: Expression) extends UnaryMathExpression(math.tan, "TAN")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the cotangent of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns the cotangent of `expr`, as if computed by
+ `1/java.lang.Math._FUNC_`.
--- End diff --
yes
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
Sorry, I missed these comments. As I understood we fix all of them here. I am just struggling with the R documentation: it seems the generated doc is incorrect even if I just take the latest commit as it is. I thought about writing to the dev list to see if others have experienced this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169299563
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -252,7 +255,14 @@ case class Ceil(child: Expression) extends UnaryMathExpression(math.ceil, "CEIL"
}
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the cosine of `expr`.",
+ usage = """
+ _FUNC_(expr) - Returns the cosine of `expr`, as if computed by
+ `java.lang.Math._FUNC_`.
+ """,
+ arguments = """
+ Arguments:
+ * expr - angle in radians
+ """,
--- End diff --
Let's remote two spaces ->
```
* expr - angle in radians
""",
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87546 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87546/testReport)** for PR 20618 at commit [`168d7b3`](https://github.com/apache/spark/commit/168d7b3715c8ad19b68c9542ffdb90bcb126c312).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87883 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87883/testReport)** for PR 20618 at commit [`10afda5`](https://github.com/apache/spark/commit/10afda5d97146af7e02cc3b18968f89f074e093c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/87860/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20618
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182585
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -916,7 +969,6 @@ case class ShiftRightUnsigned(left: Expression, right: Expression)
case class Hypot(left: Expression, right: Expression)
extends BinaryMathExpression(math.hypot, "HYPOT")
-
--- End diff --
Ditto for revert
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20618
**[Test build #87798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/87798/testReport)** for PR 20618 at commit [`fcf1338`](https://github.com/apache/spark/commit/fcf1338a3345c3f57f274f0e9bea813a24fbe581).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169313008
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +272,10 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
--- End diff --
Yes, we should
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20618: [SPARK-23329][SQL] Fix documentation of trigonometric fu...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on the issue:
https://github.com/apache/spark/pull/20618
As discussed in email R documentation is reorganized and math functions are grouped as part of SPARK-20889. Because of this grouping I dont think this change is really applicable on R. @srowen, @HyukjinKwon, @felixcheung what do you think about it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182420
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -548,7 +575,13 @@ case class Sqrt(child: Expression) extends UnaryMathExpression(math.sqrt, "SQRT"
case class Tan(child: Expression) extends UnaryMathExpression(math.tan, "TAN")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the cotangent of `expr`.",
+ usage = "_FUNC_(expr) - Returns the cotangent of `expr` ," +
--- End diff --
Ditto for formatting
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169300055
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +272,10 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
+ arguments = """
+ Arguments:
+ * expr - hyperbolic angle.
--- End diff --
not a big deal but let's remove the trailing `.` for consistency.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169213439
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1313,131 +1313,165 @@ object functions {
//////////////////////////////////////////////////////////////////////////////////////////////
/**
- * Computes the cosine inverse of the given value; the returned angle is in the range
- * 0.0 through pi.
--- End diff --
I am sorry if I missed a discussion about this. Why don't we keep this simple description for this method rather than removing out?
If we go this way, we should update both `functions.py` and `functions.R`. If you strongly prefer this, I would like to do this separately in another PR for both together.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r168521316
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -262,6 +285,11 @@ case class Cos(child: Expression) extends UnaryMathExpression(math.cos, "COS")
@ExpressionDescription(
usage = "_FUNC_(expr) - Returns the hyperbolic cosine of `expr`.",
+ arguments =
+ """
+ Arguments:
+ * expr - number whose hyperbolic consine is to be returned.
--- End diff --
I was not very familiar with hyperbolic functions so I followed the way this is described in java.lang.Math. But hyperbolic angle is really what this parameter actually is.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169182146
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -512,16 +522,27 @@ case class Rint(child: Expression) extends UnaryMathExpression(math.rint, "ROUND
case class Signum(child: Expression) extends UnaryMathExpression(math.signum, "SIGNUM")
@ExpressionDescription(
- usage = "_FUNC_(expr) - Returns the sine of `expr`.",
+ usage = "_FUNC_(expr) - Returns the sine of `expr`, as if computed by `java.lang.Math._FUNC_`.",
+ arguments =
+ """
--- End diff --
Can we have the same format for it and the same instances - https://github.com/apache/spark/pull/20618#discussion_r168670009?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20618: [SPARK-23329][SQL] Fix documentation of trigonome...
Posted by misutoth <gi...@git.apache.org>.
Github user misutoth commented on a diff in the pull request:
https://github.com/apache/spark/pull/20618#discussion_r169186706
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala ---
@@ -538,8 +559,14 @@ case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH"
""")
case class Sqrt(child: Expression) extends UnaryMathExpression(math.sqrt, "SQRT")
+// scalastyle:off line.size.limit
--- End diff --
Turned it on for all trigonometric functions.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org