[SPARK-23642][DOCS] AccumulatorV2 subclass isZero scaladoc fix #20790
Conversation
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.
Fixed line length style errors in Accumulator isZero scaladoc.
| @@ -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. | |||
| * | |||
There was a problem hiding this comment.
I think this duplicates the doc from AccumulatorV2.isZero. Can we simply remove this wrong doc and revert other changes so that we can reuse inherited doc from AccumulatorV2.isZero in all places?
There was a problem hiding this comment.
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.
|
Shall we fix the title to |
|
Wait .. I just found you opened a JIRA - SPARK-23642. Please link it by |
smallory
changed the title
|
Thanks for the pointer on the title convention, the way the contributing doc distinguished code and documentation changes left me a bit puzzled as to what actually applied to this change. |
|
ok to test |
|
Test build #88220 has finished for PR 20790 at commit
|
|
retest this please |
|
Test build #88227 has finished for PR 20790 at commit
|
|
Merged to master and branch-2.3. |
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. Author: smallory <s.mallory@gmail.com> Closes #20790 from smallory/patch-1. (cherry picked from commit 4f5bad6) Signed-off-by: hyukjinkwon <gurwls223@gmail.com>
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. Author: smallory <s.mallory@gmail.com> Closes apache#20790 from smallory/patch-1.
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. Author: smallory <s.mallory@gmail.com> Closes apache#20790 from smallory/patch-1. (cherry picked from commit 4f5bad6) Signed-off-by: hyukjinkwon <gurwls223@gmail.com>
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.