Skip to content

[SPARK-20232][Python] Improve combineByKey docs - #17545

Closed
dgingrich wants to merge 1 commit into
apache:masterfrom
dgingrich:topic-spark-20232-combinebykey-docs
Closed

[SPARK-20232][Python] Improve combineByKey docs#17545
dgingrich wants to merge 1 commit into
apache:masterfrom
dgingrich:topic-spark-20232-combinebykey-docs

Conversation

@dgingrich

Copy link
Copy Markdown

What changes were proposed in this pull request?

Improve combineByKey documentation:

  • Add note on memory allocation
  • Change example code to use different mergeValue and mergeCombiners

How was this patch tested?

Doctest.

Legal

This is my original work and I license the work to the project under the project’s open source license.

* Add note on memory allocation
* Change example code to use different mergeValue and mergeCombiners
@SparkQA

Copy link
Copy Markdown

Test build #3643 has finished for PR 17545 at commit 74691d1.

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

@holdenk

Copy link
Copy Markdown
Contributor

So the associated JIRA says "Doesn't include note about memory allocation (on aggregateBykey)" - what did you want to add in aggregateBykey?

@dgingrich

Copy link
Copy Markdown
Author

The note meant that combineByKey doesn't include the comment about memory allocation that's on aggregateByKey. I did add the memory allocation note to combineByKey, starting on line 1810.

@holdenk

Copy link
Copy Markdown
Contributor

Ok sounds good, I just wanted to make sure since the JIRA had it in two separate bullet points and I wasn't sure if it was a second thing you wanted to talk about.

LGTM.

@holdenk

Copy link
Copy Markdown
Contributor

Merged to master, thanks for working on this @dgingrich

peter-toth pushed a commit to peter-toth/spark that referenced this pull request Oct 6, 2018
## What changes were proposed in this pull request?
Improve combineByKey documentation:
* Add note on memory allocation
* Change example code to use different mergeValue and mergeCombiners
## How was this patch tested?
Doctest.
## Legal
This is my original work and I license the work to the project under the project’s open source license.
Author: David Gingrich <david@textio.com>
Closesapache#17545 from dgingrich/topic-spark-20232-combinebykey-docs.
Sign up for freeto 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

@dgingrich@SparkQA@holdenk