Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-3359][BUILD][DOCS] More changes to resolve javadoc 8 errors that will help unidoc/genjavadoc compatibility #15999

Closed
wants to merge 11 commits into from

Conversation

HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Nov 24, 2016

What changes were proposed in this pull request?

This PR only tries to fix things that looks pretty straightforward and were fixed in other previous PRs before.

This PR roughly fixes several things as below:

  • Fix unrecognisable class and method links in javadoc by changing it from [[..]] to `...`

    [error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/DataStreamReader.java:226: error: reference not found
    [error]    * Loads text files and returns a {@link DataFrame} whose schema starts with a string column named
    
  • Fix an exception annotation and remove code backticks in @throws annotation

    Currently, sbt unidoc with Java 8 complains as below:

    [error] .../java/org/apache/spark/sql/streaming/StreamingQuery.java:72: error: unexpected text
    [error]    * @throws StreamingQueryException, if <code>this</code> query has terminated with an exception.
    

    @throws should specify the correct class name from StreamingQueryException, to StreamingQueryException without backticks. (see JDK-8007644).

  • Fix [[http..]] to <a href="http..."></a>.

    -   * [[https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https Oracle
    -   * blog page]].
    +   * <a href="https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https">
    +   * Oracle blog page</a>.

    [[http...]] link markdown in scaladoc is unrecognisable in javadoc.

  • It seems class can't have @return annotation. So, two cases of this were removed.

    [error] .../java/org/apache/spark/mllib/regression/IsotonicRegression.java:27: error: invalid use of @return
    [error]    * @return New instance of IsotonicRegression.
    
  • Fix < to &lt; and > to &gt; according to HTML rules. This should be @literal{<} or something like greater than.

  • Fix </p> complaint

  • Exclude unrecognisable in javadoc, @constructor, @todo and @groupname.

How was this patch tested?

Manually tested by jekyll build with Java 7 and 8

java version "1.7.0_80"
Java(TM) SE Runtime Environment (build 1.7.0_80-b15)
Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)

Note: this does not yet make sbt unidoc suceed with Java 8 yet but it reduces the number of errors with Java 8.

@HyukjinKwon
Copy link
Member Author

I should double check the built javadoc. Will leave some images in the changes. BTW, this still does not fully resolve the problem (cc @srowen).

@SparkQA
Copy link

SparkQA commented Nov 24, 2016

Test build #69105 has finished for PR 15999 at commit cfce0e8.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 24, 2016

Test build #69109 has started for PR 15999 at commit cfce0e8.

@HyukjinKwon
Copy link
Member Author

retest this please

*/
@Since("1.3.0")
class IsotonicRegression private (private var isotonic: Boolean) extends Serializable {

/**
* Constructs IsotonicRegression instance with default parameter isotonic = true.
*
* @return New instance of IsotonicRegression.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

javadoc complains about @return for class.

[error] .../java/org/apache/spark/mllib/regression/IsotonicRegression.java:27: error: invalid use of @return
[error]    * @return New instance of IsotonicRegression.
[error]      ^

* `JavaPairRDD<String, byte[]> rdd = sparkContext.dataStreamFiles("hdfs://a-hdfs-path")`,
* <code>
* JavaPairRDD&lt;String, byte[]&gt; rdd = sparkContext.dataStreamFiles("hdfs://a-hdfs-path")
* </code>,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`JavaPairRDD&lt;String, byte[]&gt; rdd = sparkContext.dataStreamFiles("hdfs://a-hdfs-path")` prints

JavaPairRDD&lt;String, byte[]&gt; rdd = ... in scaladoc

JavaPairRDD<String, byte[]> rdd = ... in javadoc

So, I had to use <code>...</code>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to myself. It still prints the codes as above.

If we want to use < or >, we might better always wrap this with

{{{
...
}}}

rather than backticks or <code>...</code>.

@@ -1513,7 +1513,7 @@ private class LogisticAggregator(
}

/**
* When maxMargin > 0, the original formula could cause overflow.
* When maxMargin &gt; 0, the original formula could cause overflow.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

javadoc does not allow < and > as plain. This complains as below:

[error] .../spark/mllib/target/java/org/apache/spark/ml/classification/LogisticAggregator.java:127: error: bad use of '>'
[error]  * Fortunately, when max(margins) = maxMargin > 0, the loss function and the multiplier can easily
[error]                                               ^

@@ -289,7 +289,6 @@ object MultilayerPerceptronClassifier
* @param uid uid
* @param layers array of layer sizes including input and output layers
* @param weights the weights of layers
* @return prediction model
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -1283,7 +1283,7 @@ class BinaryLogisticRegressionSummary private[classification] (
* P(y_i=K-1|\vec{x}_i, \beta) = \frac{e^{\vec{x}_i^T \vec{\beta}_{K-1}}\,}{\sum_{k=0}^{K-1}
* e^{\vec{x}_i^T \vec{\beta}_k}}
* $$
* </blockquote></p>
* </blockquote>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

javadoc complains about this: error: unexpected end tag: </p>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, they are still printed fine.

I manually changed the access modifier of the same instance with this and manually tested with jekyll build.

With the codes:

2016-11-22 5 41 37

Before

2016-11-22 5 43 43

After

2016-11-22 5 51 24

They are identical.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, this is being printed weirdly in javadoc regardless of this change:

2016-11-22 5 41 17

@@ -2354,7 +2354,7 @@ private[spark] object Utils extends Logging {
* A spark url (`spark://host:port`) is a special URI that its scheme is `spark` and only contains
* host and port.
*
* @throws SparkException if `sparkUrl` is invalid.
* @note Throws `SparkException` if sparkUrl is invalid.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SparkException reference is not found in javadoc.

[error] .../java/org/apache/spark/util/Utils.java:841: error: reference not found
[error]    * @throws SparkException if sparkUrl is invalid.
[error]      ^

I am not too sure using @note instead is right.

"-tag", "tparam:X",
"-tag", "constructor:X",
"-tag", "todo:X",
"-tag", "groupname:X"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tags, @ constructor, @ todo and @groupname were excluded as they are unrecognisable in javadoc.

<tag>
<name>groupname</name>
<placement>X</placement>
</tag>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* [[https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https Oracle
* blog page]].
* <a href="https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https">
* Oracle blog page</a>.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For hyperlinks, it seems

these are fine (try it):

<a href="https://.../...">link ABC</a>
<a href="https://.../...">
link ABC</a>
<a href="https://.../...">link
ABC</a>

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heroic effort! LGTM

* @see [[http://en.wikipedia.org/wiki/Latent_Dirichlet_allocation Latent Dirichlet allocation
* (Wikipedia)]]
* @see <a href="http://en.wikipedia.org/wiki/Latent_Dirichlet_allocation">
* Latent Dirichlet allocation (Wikipedia)</a>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I observed that there are three cases of the indentations for @see.

@see ...
...
@see ...
     ...
@see ...
    ...

I just tried to match those up to @note in the way I did before which is the first case.

@SparkQA
Copy link

SparkQA commented Nov 24, 2016

Test build #69119 has finished for PR 15999 at commit cfce0e8.

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

@HyukjinKwon
Copy link
Member Author

Oh thank you! Will double check and change the title tomorrow.

@SparkQA
Copy link

SparkQA commented Nov 24, 2016

Test build #69122 has finished for PR 15999 at commit b0c805c.

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

@srowen
Copy link
Member

srowen commented Nov 24, 2016

Let's get this in for 2.1, even if it doesn't fix 100% of the errors. This looks like a big step towards making it work with javadoc 8. Let me know when ready @HyukjinKwon

* Loads a JSON file ([[http://jsonlines.org/ JSON Lines text format or newline-delimited JSON]])
* and returns the result as a [[DataFrame]].
* Loads a JSON file (<a href="http://jsonlines.org/">JSON Lines text format or
* newline-delimited JSON</a>) and returns the result as a [[DataFrame]].
Copy link
Member Author

@HyukjinKwon HyukjinKwon Nov 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to myself - It seems this is fine.

Java
2016-11-25 5 35 23

Scala

2016-11-25 5 35 45

* `JavaPairRDD<String, byte[]> rdd = sparkContext.dataStreamFiles("hdfs://a-hdfs-path")`,
* {{{
* JavaPairRDD<String, byte[]> rdd = sparkContext.dataStreamFiles("hdfs://a-hdfs-path")
* }}},
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to myself - see #15999 (comment). It seems we always should use {{{...}}} instead of backticks or <code>...</code> to print out < and > for both scaladoc and javadoc.

It is fine now.

2016-11-25 5 40 31

2016-11-25 5 41 09

@HyukjinKwon HyukjinKwon changed the title [WIP][SPARK-3359][BUILD][DOCS] More changes to resolve javadoc 8 errors that will help unidoc/genjavadoc compatibility [SPARK-3359][BUILD][DOCS] More changes to resolve javadoc 8 errors that will help unidoc/genjavadoc compatibility Nov 25, 2016
@HyukjinKwon
Copy link
Member Author

I just double-checked and I think it is ready. Thank you @srowen.

@SparkQA
Copy link

SparkQA commented Nov 25, 2016

Test build #69153 has finished for PR 15999 at commit 297783d.

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

@srowen
Copy link
Member

srowen commented Nov 25, 2016

Merged to master/2.1

asfgit pushed a commit that referenced this pull request Nov 25, 2016
…at will help unidoc/genjavadoc compatibility

## What changes were proposed in this pull request?

This PR only tries to fix things that looks pretty straightforward and were fixed in other previous PRs before.

This PR roughly fixes several things as below:

- Fix unrecognisable class and method links in javadoc by changing it from `[[..]]` to `` `...` ``

  ```
  [error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/DataStreamReader.java:226: error: reference not found
  [error]    * Loads text files and returns a {link DataFrame} whose schema starts with a string column named
  ```

- Fix an exception annotation and remove code backticks in `throws` annotation

  Currently, sbt unidoc with Java 8 complains as below:

  ```
  [error] .../java/org/apache/spark/sql/streaming/StreamingQuery.java:72: error: unexpected text
  [error]    * throws StreamingQueryException, if <code>this</code> query has terminated with an exception.
  ```

  `throws` should specify the correct class name from `StreamingQueryException,` to `StreamingQueryException` without backticks. (see [JDK-8007644](https://bugs.openjdk.java.net/browse/JDK-8007644)).

- Fix `[[http..]]` to `<a href="http..."></a>`.

  ```diff
  -   * [[https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https Oracle
  -   * blog page]].
  +   * <a href="https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https">
  +   * Oracle blog page</a>.
  ```

   `[[http...]]` link markdown in scaladoc is unrecognisable in javadoc.

- It seems class can't have `return` annotation. So, two cases of this were removed.

  ```
  [error] .../java/org/apache/spark/mllib/regression/IsotonicRegression.java:27: error: invalid use of return
  [error]    * return New instance of IsotonicRegression.
  ```

- Fix < to `&lt;` and > to `&gt;` according to HTML rules.

- Fix `</p>` complaint

- Exclude unrecognisable in javadoc, `constructor`, `todo` and `groupname`.

## How was this patch tested?

Manually tested by `jekyll build` with Java 7 and 8

```
java version "1.7.0_80"
Java(TM) SE Runtime Environment (build 1.7.0_80-b15)
Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)
```

```
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)
```

Note: this does not yet make sbt unidoc suceed with Java 8 yet but it reduces the number of errors with Java 8.

Author: hyukjinkwon <[email protected]>

Closes #15999 from HyukjinKwon/SPARK-3359-errors.

(cherry picked from commit 51b1c15)
Signed-off-by: Sean Owen <[email protected]>
@asfgit asfgit closed this in 51b1c15 Nov 25, 2016
robert3005 pushed a commit to palantir/spark that referenced this pull request Dec 2, 2016
…at will help unidoc/genjavadoc compatibility

## What changes were proposed in this pull request?

This PR only tries to fix things that looks pretty straightforward and were fixed in other previous PRs before.

This PR roughly fixes several things as below:

- Fix unrecognisable class and method links in javadoc by changing it from `[[..]]` to `` `...` ``

  ```
  [error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/DataStreamReader.java:226: error: reference not found
  [error]    * Loads text files and returns a {link DataFrame} whose schema starts with a string column named
  ```

- Fix an exception annotation and remove code backticks in `throws` annotation

  Currently, sbt unidoc with Java 8 complains as below:

  ```
  [error] .../java/org/apache/spark/sql/streaming/StreamingQuery.java:72: error: unexpected text
  [error]    * throws StreamingQueryException, if <code>this</code> query has terminated with an exception.
  ```

  `throws` should specify the correct class name from `StreamingQueryException,` to `StreamingQueryException` without backticks. (see [JDK-8007644](https://bugs.openjdk.java.net/browse/JDK-8007644)).

- Fix `[[http..]]` to `<a href="http..."></a>`.

  ```diff
  -   * [[https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https Oracle
  -   * blog page]].
  +   * <a href="https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https">
  +   * Oracle blog page</a>.
  ```

   `[[http...]]` link markdown in scaladoc is unrecognisable in javadoc.

- It seems class can't have `return` annotation. So, two cases of this were removed.

  ```
  [error] .../java/org/apache/spark/mllib/regression/IsotonicRegression.java:27: error: invalid use of return
  [error]    * return New instance of IsotonicRegression.
  ```

- Fix < to `&lt;` and > to `&gt;` according to HTML rules.

- Fix `</p>` complaint

- Exclude unrecognisable in javadoc, `constructor`, `todo` and `groupname`.

## How was this patch tested?

Manually tested by `jekyll build` with Java 7 and 8

```
java version "1.7.0_80"
Java(TM) SE Runtime Environment (build 1.7.0_80-b15)
Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)
```

```
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)
```

Note: this does not yet make sbt unidoc suceed with Java 8 yet but it reduces the number of errors with Java 8.

Author: hyukjinkwon <[email protected]>

Closes apache#15999 from HyukjinKwon/SPARK-3359-errors.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
…at will help unidoc/genjavadoc compatibility

## What changes were proposed in this pull request?

This PR only tries to fix things that looks pretty straightforward and were fixed in other previous PRs before.

This PR roughly fixes several things as below:

- Fix unrecognisable class and method links in javadoc by changing it from `[[..]]` to `` `...` ``

  ```
  [error] .../spark/sql/core/target/java/org/apache/spark/sql/streaming/DataStreamReader.java:226: error: reference not found
  [error]    * Loads text files and returns a {link DataFrame} whose schema starts with a string column named
  ```

- Fix an exception annotation and remove code backticks in `throws` annotation

  Currently, sbt unidoc with Java 8 complains as below:

  ```
  [error] .../java/org/apache/spark/sql/streaming/StreamingQuery.java:72: error: unexpected text
  [error]    * throws StreamingQueryException, if <code>this</code> query has terminated with an exception.
  ```

  `throws` should specify the correct class name from `StreamingQueryException,` to `StreamingQueryException` without backticks. (see [JDK-8007644](https://bugs.openjdk.java.net/browse/JDK-8007644)).

- Fix `[[http..]]` to `<a href="http..."></a>`.

  ```diff
  -   * [[https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https Oracle
  -   * blog page]].
  +   * <a href="https://blogs.oracle.com/java-platform-group/entry/diagnosing_tls_ssl_and_https">
  +   * Oracle blog page</a>.
  ```

   `[[http...]]` link markdown in scaladoc is unrecognisable in javadoc.

- It seems class can't have `return` annotation. So, two cases of this were removed.

  ```
  [error] .../java/org/apache/spark/mllib/regression/IsotonicRegression.java:27: error: invalid use of return
  [error]    * return New instance of IsotonicRegression.
  ```

- Fix < to `&lt;` and > to `&gt;` according to HTML rules.

- Fix `</p>` complaint

- Exclude unrecognisable in javadoc, `constructor`, `todo` and `groupname`.

## How was this patch tested?

Manually tested by `jekyll build` with Java 7 and 8

```
java version "1.7.0_80"
Java(TM) SE Runtime Environment (build 1.7.0_80-b15)
Java HotSpot(TM) 64-Bit Server VM (build 24.80-b11, mixed mode)
```

```
java version "1.8.0_45"
Java(TM) SE Runtime Environment (build 1.8.0_45-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.45-b02, mixed mode)
```

Note: this does not yet make sbt unidoc suceed with Java 8 yet but it reduces the number of errors with Java 8.

Author: hyukjinkwon <[email protected]>

Closes apache#15999 from HyukjinKwon/SPARK-3359-errors.
asfgit pushed a commit that referenced this pull request Apr 12, 2017
## What changes were proposed in this pull request?

This PR proposes to run Spark unidoc to test Javadoc 8 build as Javadoc 8 is easily re-breakable.

There are several problems with it:

- It introduces little extra bit of time to run the tests. In my case, it took 1.5 mins more (`Elapsed :[94.8746569157]`). How it was tested is described in "How was this patch tested?".

- > One problem that I noticed was that Unidoc appeared to be processing test sources: if we can find a way to exclude those from being processed in the first place then that might significantly speed things up.

  (see  joshrosen's [comment](https://issues.apache.org/jira/browse/SPARK-18692?focusedCommentId=15947627&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15947627))

To complete this automated build, It also suggests to fix existing Javadoc breaks / ones introduced by test codes as described above.

There fixes are similar instances that previously fixed. Please refer #15999 and #16013

Note that this only fixes **errors** not **warnings**. Please see my observation #17389 (comment) for spurious errors by warnings.

## How was this patch tested?

Manually via `jekyll build` for building tests. Also, tested via running `./dev/run-tests`.

This was tested via manually adding `time.time()` as below:

```diff
     profiles_and_goals = build_profiles + sbt_goals

     print("[info] Building Spark unidoc (w/Hive 1.2.1) using SBT with these arguments: ",
           " ".join(profiles_and_goals))

+    import time
+    st = time.time()
     exec_sbt(profiles_and_goals)
+    print("Elapsed :[%s]" % str(time.time() - st))
```

produces

```
...
========================================================================
Building Unidoc API Documentation
========================================================================
...
[info] Main Java API documentation successful.
...
Elapsed :[94.8746569157]
...

Author: hyukjinkwon <[email protected]>

Closes #17477 from HyukjinKwon/SPARK-18692.
ghost pushed a commit to dbtsai/spark that referenced this pull request Jun 29, 2017
PR apache#15999 included fixes for doc strings in the ML shared param traits (occurrences of `>` and `>=`).

This PR simply uses the HTML-escaped version of the param doc to embed into the Scaladoc, to ensure that when `SharedParamsCodeGen` is run, the generated javadoc will be compliant for Java 8.

## How was this patch tested?
Existing tests

Author: Nick Pentreath <[email protected]>

Closes apache#18420 from MLnick/shared-params-javadoc8.
asfgit pushed a commit that referenced this pull request Jun 29, 2017
PR #15999 included fixes for doc strings in the ML shared param traits (occurrences of `>` and `>=`).

This PR simply uses the HTML-escaped version of the param doc to embed into the Scaladoc, to ensure that when `SharedParamsCodeGen` is run, the generated javadoc will be compliant for Java 8.

## How was this patch tested?
Existing tests

Author: Nick Pentreath <[email protected]>

Closes #18420 from MLnick/shared-params-javadoc8.

(cherry picked from commit 70085e8)
Signed-off-by: Sean Owen <[email protected]>
robert3005 pushed a commit to palantir/spark that referenced this pull request Jun 29, 2017
PR apache#15999 included fixes for doc strings in the ML shared param traits (occurrences of `>` and `>=`).

This PR simply uses the HTML-escaped version of the param doc to embed into the Scaladoc, to ensure that when `SharedParamsCodeGen` is run, the generated javadoc will be compliant for Java 8.

## How was this patch tested?
Existing tests

Author: Nick Pentreath <[email protected]>

Closes apache#18420 from MLnick/shared-params-javadoc8.
@HyukjinKwon HyukjinKwon deleted the SPARK-3359-errors branch January 2, 2018 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants