Skip to content

[SPARK-25328][PYTHON] Add an example for having two columns as the grouping key in group aggregate pandas UDF #22329

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.

Sign up for GitHub

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

Jump to bottom
Closed
HyukjinKwon wants to merge 3 commits into from
Closed

[SPARK-25328][PYTHON] Add an example for having two columns as the grouping key in group aggregate pandas UDF #22329

HyukjinKwon wants to merge 3 commits into from

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Sep 4, 2018 •
edited
Loading

What changes were proposed in this pull request?

This PR proposes to add another example for multiple grouping key in group aggregate pandas UDF since this feature could make users still confused.

How was this patch tested?

Manually tested and documentation built.

@SparkQA
Copy link

SparkQA commented Sep 4, 2018

Test build #95666 has finished for PR 22329 at commit 36a7ccc.

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

icexelloss
icexelloss reviewed Sep 4, 2018
View reviewed changes
python/pyspark/sql/functions.py Outdated
| 1|1.5|
| 2|6.0|
+---+---+
>>> @pandas_udf("id long, v1 double, v2 double", PandasUDFType.GROUPED_MAP) # doctest: +SKIP
Copy link
Contributor

@icexelloss icexelloss Sep 4, 2018

Choose a reason for hiding this comment

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

It took me a while to realize v1 is a grouping key. It also a bit uncommon to use double value as a grouping key . How about we do sth like?

id long, additional_key long, v double

@SparkQA
Copy link

SparkQA commented Sep 5, 2018

Test build #95690 has finished for PR 22329 at commit 2ad350c.

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

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Sep 5, 2018

cc @gatorsmile and @BryanCutler

icexelloss
icexelloss reviewed Sep 5, 2018
View reviewed changes
python/pyspark/sql/functions.py Outdated
| 2|6.0|
+---+---+
>>> @pandas_udf(
... "id long, additional_key double, v double",
Copy link
Contributor

@icexelloss icexelloss Sep 5, 2018

Choose a reason for hiding this comment

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

do you mind changing the type of additional_key to long? It seems like the type coercion here is not necessary.

Copy link
Member

@BryanCutler BryanCutler Sep 5, 2018

Choose a reason for hiding this comment

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

Sorry, I know you just changed it, but I think just naming the column "ceil(v1 / 2)" with a type long would be a little more clear. Although "additional_key" is ok too, if you guys want to keep that.

@SparkQA
Copy link

SparkQA commented Sep 6, 2018

Test build #95734 has finished for PR 22329 at commit 1f342aa.

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

BryanCutler
BryanCutler approved these changes Sep 6, 2018
View reviewed changes
Copy link
Member

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

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

LGTM

@icexelloss
Copy link
Contributor

icexelloss commented Sep 6, 2018

LGTM

@asfgit asfgit closed this in 7ef6d1d Sep 6, 2018
@BryanCutler
Copy link
Member

BryanCutler commented Sep 6, 2018

merged to master, thanks @HyukjinKwon . I just saw branch-2.4 was cut already, I'll see if I can figure out how to merge there too.

asfgit pushed a commit that referenced this pull request Sep 6, 2018
@HyukjinKwon @BryanCutler
[SPARK-25328][PYTHON] Add an example for having two columns as the gr…
f2d5022 
…ouping key in group aggregate pandas UDF

## What changes were proposed in this pull request?

This PR proposes to add another example for multiple grouping key in group aggregate pandas UDF since this feature could make users still confused.

## How was this patch tested?

Manually tested and documentation built.

Closes #22329 from HyukjinKwon/SPARK-25328.

Authored-by: hyukjinkwon <[email protected]>
Signed-off-by: Bryan Cutler <[email protected]>
(cherry picked from commit 7ef6d1d)
Signed-off-by: Bryan Cutler <[email protected]>
@BryanCutler
Copy link
Member

BryanCutler commented Sep 6, 2018

merged to branch-2.4

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Sep 7, 2018

Thanks guys :-)

@HyukjinKwon HyukjinKwon deleted the SPARK-25328 branch October 16, 2018 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BryanCutler BryanCutler BryanCutler approved these changes

+1 more reviewer

@icexelloss icexelloss icexelloss left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HyukjinKwon @SparkQA @icexelloss @BryanCutler