Skip to content

[SPARK-15866] Rename listAccumulator collectionAccumulator#13594

Closed
rxin wants to merge 1 commit intoapache:masterfrom
rxin:SPARK-15866
Closed

[SPARK-15866] Rename listAccumulator collectionAccumulator#13594
rxin wants to merge 1 commit intoapache:masterfrom
rxin:SPARK-15866

Conversation

@rxin
Copy link
Contributor

@rxin rxin commented Jun 10, 2016

What changes were proposed in this pull request?

SparkContext.listAccumulator, by Spark's convention, makes it sound like "list" is a verb and the method should return a list of accumulators. This patch renames the method and the class collection accumulator.

How was this patch tested?

Updated test case to reflect the names.

@rxin
Copy link
Contributor Author

rxin commented Jun 10, 2016

cc @cloud-fan

*
* @since 2.0.0
*/
class CollectionAccumulator[T] extends AccumulatorV2[T, java.util.List[T]] {
Copy link
Member

Choose a reason for hiding this comment

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

Question: so this method still accumulates a list, despite its name (which does sound better). Can an accumulator guarantee an order to the items add to the list meaningfully? if not, then maybe it should be advertised as an accumulator of a Collection.

Copy link
Contributor

@cloud-fan cloud-fan Jun 10, 2016

Choose a reason for hiding this comment

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

the order is preserved for the inputs from same partition, but not among partitions.

@SparkQA
Copy link

SparkQA commented Jun 10, 2016

Test build #60282 has finished for PR 13594 at commit 867efed.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class CollectionAccumulator[T] extends AccumulatorV2[T, java.util.List[T]]

@cloud-fan
Copy link
Contributor

LGTM

asfgit pushed a commit that referenced this pull request Jun 10, 2016
## What changes were proposed in this pull request?
SparkContext.listAccumulator, by Spark's convention, makes it sound like "list" is a verb and the method should return a list of accumulators. This patch renames the method and the class collection accumulator.

## How was this patch tested?
Updated test case to reflect the names.

Author: Reynold Xin <rxin@databricks.com>

Closes #13594 from rxin/SPARK-15866.

(cherry picked from commit 254bc8c)
Signed-off-by: Reynold Xin <rxin@databricks.com>
@asfgit asfgit closed this in 254bc8c Jun 10, 2016
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.

4 participants