Skip to content
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.

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

[SPARK-13479] [SQL] [PYTHON] Added Python API for approxQuantile #11356

Closed
wants to merge 4 commits into from

Conversation

jkbradley
Copy link
Member

What changes were proposed in this pull request?

  • Scala DataFrameStatFunctions: Added version of approxQuantile taking a List instead of an Array, for Python compatbility
  • Python DataFrame and DataFrameStatFunctions: Added approxQuantile

How was this patch tested?

  • unit test in sql/tests.py

Documentation was copied from the existing approxQuantile exactly.

@jkbradley jkbradley changed the title Added Python API for approxQuantile [SPARK-13479] [SQL] [PYTHON] Added Python API for approxQuantile Feb 25, 2016
@jkbradley
Copy link
Member Author

CC: @mengxr @thunterdb

@SparkQA
Copy link

SparkQA commented Feb 25, 2016

Test build #51913 has finished for PR 11356 at commit e2f85b6.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

*/
def approxQuantile(
col: String,
probabilities: List[Double],
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe java.util.List to make it very explicit this is a java list, not a scala list ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, thanks

@SparkQA
Copy link

SparkQA commented Feb 25, 2016

Test build #51916 has finished for PR 11356 at commit 770cb4c.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jkbradley
Copy link
Member Author

Weird...the method works for me locally.

@jkbradley
Copy link
Member Author

oh nevermind, I didn't test the last commit properly. I'll send a fix

* Note that values greater than 1 are accepted but give the same result as 1.
* @return the approximate quantiles at the given probabilities
*
* @since 2.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a package private API. Maybe we should simply say "Python-friendly version of [[...]].`.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would have, but I was just following other conventions in the DataFrame code. I'll change it though.

@mengxr
Copy link
Contributor

mengxr commented Feb 25, 2016

LGTM

@SparkQA
Copy link

SparkQA commented Feb 25, 2016

Test build #51932 has finished for PR 11356 at commit 3f18d78.

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

@SparkQA
Copy link

SparkQA commented Feb 25, 2016

Test build #51933 has finished for PR 11356 at commit 4f21f06.

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

@mengxr
Copy link
Contributor

mengxr commented Feb 25, 2016

Merged into master. Thanks!

@asfgit asfgit closed this in 13ce10e Feb 25, 2016
@jkbradley jkbradley deleted the approx-quantile-python branch February 25, 2016 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants