Skip to content

Add cauchy_cdf and inverse_cauchy_cdf functions#15818

Merged
rongrong merged 1 commit intoprestodb:masterfrom
jsawruk:cauchy_dist
Jul 16, 2021
Merged

Add cauchy_cdf and inverse_cauchy_cdf functions#15818
rongrong merged 1 commit intoprestodb:masterfrom
jsawruk:cauchy_dist

Conversation

@jsawruk
Copy link
Copy Markdown
Contributor

@jsawruk jsawruk commented Mar 11, 2021

https://en.wikipedia.org/wiki/Cauchy_distribution

Test plan

./mvnw test -B -Dair.check.skip-all -Dmaven.javadoc.skip=true -DLogTestDurationListener.enabled=true --fail-at-end -pl :presto-main
== RELEASE NOTES ==

General Changes
* Add Cauchy distribution CDF and inverse CDF functions to MathFunctions.java

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 11, 2021

CLA Signed

The committers are authorized under a signed CLA.

  • ✅ jsawruk (68cdab021a3187a1700cbd9379593516200ba8bb)

@leepface
Copy link
Copy Markdown
Contributor

Code review:

  • Squash commits together to one
  • Move the URL from the commit message to the commit summary

@jsawruk jsawruk changed the title Added Cauchy distribution (https://en.wikipedia.org/wiki/Cauchy_distr… Added Cauchy distribution Mar 11, 2021
@jsawruk jsawruk changed the title Added Cauchy distribution Added Cauchy distribution (#15798) Mar 12, 2021
@jsawruk jsawruk force-pushed the cauchy_dist branch 2 times, most recently from a036d16 to 892992c Compare March 12, 2021 18:47
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 12, 2021

CLA Signed

The committers are authorized under a signed CLA.

  • ✅ jsawruk (c6aeb7134fa5ae6be428a979d9e576b6dd139b3b)

Copy link
Copy Markdown
Contributor

@rongrong rongrong left a comment

Choose a reason for hiding this comment

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

Looks good. Can you please rebase and push again? Thanks!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nits: just put these in normal paragraphs that wraps in reasonable length is fine. No need to put each sentence in a new line.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here. please follow formatting of the file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use "real value" or double, or mention real number of type double. REAL is a type so real number is a bit ambiguous.

@rongrong
Copy link
Copy Markdown
Contributor

Please change commit title and remove the reference to the PR (just Add function xxxx is enough). Also please remove the commit message body.

@jsawruk jsawruk force-pushed the cauchy_dist branch 2 times, most recently from 17c5f14 to ca406aa Compare June 15, 2021 20:48
@jsawruk jsawruk changed the title Added Cauchy distribution (#15798) Add Cauchy distribution Jun 15, 2021
@rongrong
Copy link
Copy Markdown
Contributor

Please change the commit title to Add Cauchy distribution functions

@jsawruk jsawruk changed the title Add Cauchy distribution Add cauchy_cdf and inverse_cauchy_cdf functions Jul 12, 2021
@rongrong rongrong merged commit 5633ed8 into prestodb:master Jul 16, 2021
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.

3 participants