[MINOR][SQL][DOCS] Improve unix_timestamp's scaladoc (and typo hunting) #17801

jaceklaskowski · 2017-04-28T08:20:06Z

What changes were proposed in this pull request?

Docs are consistent (across different unix_timestamp variants and their internal expressions)
typo hunting

How was this patch tested?

local build

SparkQA · 2017-04-28T08:35:24Z

Test build #76266 has finished for PR 17801 at commit 20edec8.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2017-04-28T08:36:47Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

  /**
   * Converts a date/timestamp/string to a value of string in the format specified by the date
   * format given by the second argument.
   *
   * A pattern could be for instance `dd.MM.yyyy` and could return a string like '18.03.1993'. All
-   * pattern letters of `java.text.SimpleDateFormat` can be used.
+   * pattern letters of [[https://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html java.text.SimpleDateFormat]] can be used.


We don't generally link to external javadoc, and we've had problems creating a normal javadoc link to the class in many instances. For consistency, I wouldn't bother with this.

SparkQA · 2017-04-28T09:25:14Z

Test build #76267 has finished for PR 17801 at commit c3cc96b.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

jaceklaskowski · 2017-04-28T12:01:12Z

Are the errors (that led to fails to generate documentation) after my change? Look very weird to me.

[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/scheduler/BlacklistTracker.java:117: error: ExecutorAllocationClient is not public in org.apache.spark; cannot be accessed from outside package
[error]   public   BlacklistTracker (org.apache.spark.scheduler.LiveListenerBus listenerBus, org.apache.spark.SparkConf conf, scala.Option<org.apache.spark.ExecutorAllocationClient> allocationClient, org.apache.spark.util.Clock clock)  { throw new RuntimeException(); }
[error]                                                                                                                                                    ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/scheduler/BlacklistTracker.java:118: error: ExecutorAllocationClient is not public in org.apache.spark; cannot be accessed from outside package
[error]   public   BlacklistTracker (org.apache.spark.SparkContext sc, scala.Option<org.apache.spark.ExecutorAllocationClient> allocationClient)  { throw new RuntimeException(); }
[error]                                                                                             ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/SparkConf.java:133: error: ConfigReader is not public in org.apache.spark.internal.config; cannot be accessed from outside package
[error]   private  org.apache.spark.internal.config.ConfigReader reader ()  { throw new RuntimeException(); }
[error]                                            ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/SparkConf.java:138: error: ConfigEntry is not public in org.apache.spark.internal.config; cannot be accessed from outside package
[error]    <T extends java.lang.Object> org.apache.spark.SparkConf set (org.apache.spark.internal.config.ConfigEntry<T> entry, T value)  { throw new RuntimeException(); }
[error]                                                                                                 ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/SparkConf.java:139: error: OptionalConfigEntry is not public in org.apache.spark.internal.config; cannot be accessed from outside package
[error]    <T extends java.lang.Object> org.apache.spark.SparkConf set (org.apache.spark.internal.config.OptionalConfigEntry<T> entry, T value)  { throw new RuntimeException(); }
[error]                                                                                                 ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/SparkConf.java:187: error: ConfigEntry is not public in org.apache.spark.internal.config; cannot be accessed from outside package
[error]    <T extends java.lang.Object> org.apache.spark.SparkConf setIfMissing (org.apache.spark.internal.config.ConfigEntry<T> entry, T value)  { throw new RuntimeException(); }
[error]                                                                                                          ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/SparkConf.java:188: error: OptionalConfigEntry is not public in org.apache.spark.internal.config; cannot be accessed from outside package
[error]    <T extends java.lang.Object> org.apache.spark.SparkConf setIfMissing (org.apache.spark.internal.config.OptionalConfigEntry<T> entry, T value)  { throw new RuntimeException(); }
[error]                                                                                                          ^
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/target/java/org/apache/spark/SparkConf.java:208: error: ConfigEntry is not public in org.apache.spark.internal.config; cannot be accessed from outside package
[error]     org.apache.spark.SparkConf remove (org.apache.spark.internal.config.ConfigEntry<?> entry)  { throw new RuntimeException(); }

HyukjinKwon · 2017-04-28T14:05:07Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

+   * Converts time string with given pattern to Unix timestamp (in seconds).
+   * Returns `null` if fails.
+   *
+   * @see [[http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html Customizing Formats]]


It looks the documentation generation for Javadoc 8 is being failed due to @see with link -

[error] /home/jenkins/workspace/SparkPullRequestBuilder/sql/core/target/java/org/apache/spark/sql/functions.java:2996: error: self-closing element not allowed [error] * @see <a href="http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html Customizing Formats"/> [error] ^ [error] /home/jenkins/workspace/SparkPullRequestBuilder/sql/core/target/java/org/apache/spark/sql/functions.java:2996: error: invalid uri: "http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html Customizing Formats" [error] * @see <a href="http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html Customizing Formats"/> [error] ^

Probably, we should wrap it href as I did before - #16013 or find a way to make this link properly (or just remove it).

The other errors seem spurious. Please refer my observation - #17389 (comment)

HyukjinKwon

These are all from me.

HyukjinKwon · 2017-05-06T20:52:57Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

-   * Gets current Unix timestamp in seconds.
+   * Returns the current Unix timestamp (in seconds).
+   *
+   * NOTE: All calls of `unix_timestamp` within the same query return the same value


Should we use @note and fix the corresponding contents in Python and R if applicable?

HyukjinKwon · 2017-05-06T20:59:55Z

sql/core/src/main/scala/org/apache/spark/sql/functions.scala

+   * Converts time string with given pattern to Unix timestamp (in seconds).
+   * Returns `null` if fails.
+   *
+   * @see <a href="http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html">Customizing Formats</a>


@see <a href="http://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html"> Customizing Formats</a>

Given my past try, the above one works - #15999 (comment) (IIRC, I checked the output).

that can avoid having scalastyle:off

SparkQA · 2017-05-06T22:17:55Z

Test build #76528 has finished for PR 17801 at commit 5326ad1.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
public class JavaFPGrowthExample
class SingularValueDecomposition(JavaModelWrapper):

SparkQA · 2017-05-07T18:18:20Z

Test build #76546 has finished for PR 17801 at commit c96f137.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile

LGTM

gatorsmile · 2017-05-07T20:56:30Z

Thanks! Merging to master.

## What changes were proposed in this pull request? * Docs are consistent (across different `unix_timestamp` variants and their internal expressions) * typo hunting ## How was this patch tested? local build Author: Jacek Laskowski <[email protected]> Closes apache#17801 from jaceklaskowski/unix_timestamp.

[MINOR][SQL][DOCS] Improve unix_timestamp's scaladoc (and typo hunting)

20edec8

srowen reviewed Apr 28, 2017

View reviewed changes

Review round 1

c3cc96b

HyukjinKwon reviewed Apr 28, 2017

View reviewed changes

jaceklaskowski added 2 commits May 6, 2017 21:55

Review round 2

742fbb4

Merge remote-tracking branch 'origin/master' into unix_timestamp

5326ad1

HyukjinKwon reviewed May 6, 2017

View reviewed changes

jaceklaskowski added 3 commits May 7, 2017 17:59

Review round 3

1079b7d

Merge remote-tracking branch 'origin/master' into unix_timestamp

581e8e4

Review round 3 (missed one comment)

c96f137

gatorsmile approved these changes May 7, 2017

View reviewed changes

asfgit closed this in 500436b May 7, 2017

jaceklaskowski deleted the unix_timestamp branch May 8, 2017 10:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MINOR][SQL][DOCS] Improve unix_timestamp's scaladoc (and typo hunting) #17801

[MINOR][SQL][DOCS] Improve unix_timestamp's scaladoc (and typo hunting) #17801

jaceklaskowski commented Apr 28, 2017

SparkQA commented Apr 28, 2017

srowen Apr 28, 2017

SparkQA commented Apr 28, 2017

jaceklaskowski commented Apr 28, 2017

HyukjinKwon Apr 28, 2017

HyukjinKwon left a comment

HyukjinKwon May 6, 2017

HyukjinKwon May 6, 2017

felixcheung May 7, 2017

SparkQA commented May 6, 2017

SparkQA commented May 7, 2017

gatorsmile left a comment

gatorsmile commented May 7, 2017

[MINOR][SQL][DOCS] Improve unix_timestamp's scaladoc (and typo hunting) #17801

[MINOR][SQL][DOCS] Improve unix_timestamp's scaladoc (and typo hunting) #17801

Conversation

jaceklaskowski commented Apr 28, 2017

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Apr 28, 2017

srowen Apr 28, 2017

Choose a reason for hiding this comment

SparkQA commented Apr 28, 2017

jaceklaskowski commented Apr 28, 2017

HyukjinKwon Apr 28, 2017

Choose a reason for hiding this comment

HyukjinKwon left a comment

Choose a reason for hiding this comment

HyukjinKwon May 6, 2017

Choose a reason for hiding this comment

HyukjinKwon May 6, 2017

Choose a reason for hiding this comment

felixcheung May 7, 2017

Choose a reason for hiding this comment

SparkQA commented May 6, 2017

SparkQA commented May 7, 2017

gatorsmile left a comment

Choose a reason for hiding this comment

gatorsmile commented May 7, 2017