Skip to content

Conversation

@roxannemoslehi
Copy link

@roxannemoslehi roxannemoslehi commented Mar 23, 2017

What changes were proposed in this pull request?

Updated the description for the format_number description to indicate that it uses HALF_EVEN rounding. Updated the description for the round description to indicate that it uses HALF_UP rounding.

How was this patch tested?

Just changing the two function comments so no testing involved.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@srowen
Copy link
Member

srowen commented Mar 23, 2017

Have a look at http://spark.apache.org/contributing.html as the template says.
I think you'd want to describe this a little more, and consider whether other functions could use this detail?

@roxannemoslehi
Copy link
Author

Hmm @srowen do you mean describe HALF_EVEN round mode? Also, I know some of the other functions that use this type of rounding already include it in their definition.

@srowen
Copy link
Member

srowen commented Mar 23, 2017

No I really meant fill out the PR a little more (see link). The title isn't descriptive.
If you mean this is just making the docs consistent that's great, just note that in the description here. Helps reviewers understand why you think this change should happen.

@ash211
Copy link
Contributor

ash211 commented Mar 23, 2017

Thanks for contributing to Spark @roxannemoslehi !

I think Sean just means updating the title to something more like [DOCS] Clarify round mode in format_number function. It doesn't feel big enough to be worth a Jira ticket for such a small docs change.

@rxin
Copy link
Contributor

rxin commented Mar 23, 2017

Yea we definitely need a better title. Thanks for contributing though.

@rxin
Copy link
Contributor

rxin commented Mar 24, 2017

@roxannemoslehi can you fix the title? We can then merge this.

@roxannemoslehi roxannemoslehi changed the title Update functions.scala [DOCS] Clarify round mode for format_number function Mar 24, 2017
@roxannemoslehi
Copy link
Author

Yup! Sorry about that everyone. Looks like my intended commit message didn't make it through originally but should be fixed now!

@srowen
Copy link
Member

srowen commented Mar 24, 2017

How about the doc of the round(Column) method earlier, same thing?

@roxannemoslehi
Copy link
Author

Looks like round uses HALF_UP rounding mode

@roxannemoslehi roxannemoslehi changed the title [DOCS] Clarify round mode for format_number function [DOCS] Clarify round mode for format_number & round functions Mar 24, 2017
@rxin
Copy link
Contributor

rxin commented Mar 24, 2017

Thanks - merging in master.

@asfgit asfgit closed this in f88f56b Mar 24, 2017
asfgit pushed a commit that referenced this pull request Mar 27, 2017
## What changes were proposed in this pull request?

This PR proposes to match minor documentations changes in #17399 and #17380 to R/Python.

## How was this patch tested?

Manual tests in Python , Python tests via `./python/run-tests.py --module=pyspark-sql` and lint-checks for Python/R.

Author: hyukjinkwon <[email protected]>

Closes #17429 from HyukjinKwon/minor-match-doc.
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.

5 participants