You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by smallory <gi...@git.apache.org> on 2018/03/09 22:19:55 UTC

[GitHub] spark pull request #20790: AccumulatorV2 subclass isZero scaladoc fix

GitHub user smallory opened a pull request:

    https://github.com/apache/spark/pull/20790

    AccumulatorV2 subclass isZero scaladoc fix

    Added/corrected scaladoc for isZero on the DoubleAccumulator, CollectionAccumulator, and LongAccumulator subclasses of AccumulatorV2, particularly noting where there are requirements in addition to having a value of zero in order to return true.
    
    ## What changes were proposed in this pull request?
    
    Three scaladoc comments are updated in AccumulatorV2.scala
    No changes outside of comment blocks were made.
    
    ## How was this patch tested?
    
    Running "sbt unidoc", fixing style errors found, and reviewing the resulting local scaladoc in firefox.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/smallory/spark patch-1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/20790.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 #20790
    
----
commit e165151676f412fc508e465b49ae058366da4c9d
Author: smallory <s....@...>
Date:   2018-03-09T19:49:27Z

    Updating isZero scaladoc
    
    Added/corrected scaladoc for isZero on subclasses of AccumulatorV2, particularly noting where there are requirements in addition to having a value of zero in order to return true.

commit 6656be73b49b3fe1f4d26a44bee70ea7d2841e38
Author: smallory <s....@...>
Date:   2018-03-09T22:09:34Z

    Accumulator isZero scaladoc line length fix
    
    Fixed line length style errors in Accumulator isZero scaladoc.

----


---

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

[GitHub] spark issue #20790: AccumulatorV2 subclass isZero scaladoc fix

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    Shall we fix the title to `[MINOR][DOCS] AccumulatorV2  ...` to be consistent with other PRs?


---

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

[GitHub] spark pull request #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero...

Posted by smallory <gi...@git.apache.org>.

Github user smallory commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20790#discussion_r174225748
  
    --- Diff: core/src/main/scala/org/apache/spark/util/AccumulatorV2.scala ---
    @@ -290,7 +290,8 @@ class LongAccumulator extends AccumulatorV2[jl.Long, jl.Long] {
       private var _count = 0L
     
       /**
    -   * Adds v to the accumulator, i.e. increment sum by v and count by 1.
    +   * Returns false if this accumulator has had any values added to it or the sum is non-zero.
    +   *
    --- End diff --
    
    The current documentation for AccumulatorV2.isZero would be misleading for the behaviour shown when values have been added to the accumulator, but the sum is zero. This still would return false, even though it is a non-count accumulator. I don't believe that any of the implementations in this file actually behave exactly as described by AccumulatorV2.isZero.


---

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

[GitHub] spark issue #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    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 #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    **[Test build #88227 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88227/testReport)** for PR 20790 at commit [`6656be7`](https://github.com/apache/spark/commit/6656be73b49b3fe1f4d26a44bee70ea7d2841e38).


---

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

[GitHub] spark issue #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    retest this please


---

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

[GitHub] spark issue #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    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 #20790: AccumulatorV2 subclass isZero scaladoc fix

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    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 #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    ok to test


---

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

[GitHub] spark issue #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    **[Test build #88220 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88220/testReport)** for PR 20790 at commit [`6656be7`](https://github.com/apache/spark/commit/6656be73b49b3fe1f4d26a44bee70ea7d2841e38).
     * This patch **fails due to an unknown error code, -9**.
     * 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 #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    **[Test build #88227 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88227/testReport)** for PR 20790 at commit [`6656be7`](https://github.com/apache/spark/commit/6656be73b49b3fe1f4d26a44bee70ea7d2841e38).
     * 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 issue #20790: [SPARK-23642][DOCS] AccumulatorV2 subclass isZero scalad...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/20790
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88220/
    Test FAILed.


---

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