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

Creation of Exact Quantile Check #512

Merged
merged 2 commits into from
Oct 27, 2023

Conversation

jmilis2000
Copy link
Contributor

Issue #, if available:

Description of changes:
Added hasExactQuantile Check. Similar to hasApproxQuantile but finds more precise quantile value.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@rdsharma26
Copy link
Contributor

@jmilis2000 thanks for the PR! We will be having a look shortly. Can you please check in the meantime why the build is failing?

@jmilis2000
Copy link
Contributor Author

@jmilis2000 thanks for the PR! We will be having a look shortly. Can you please check in the meantime why the build is failing?

@rdsharma26 I fixed the issue. The build should work now. It failed because there was no amazon copyright at the top of the new analyzer i created.

Copy link
Contributor

@rdsharma26 rdsharma26 left a comment

Choose a reason for hiding this comment

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

LGTM. To confirm, you are using Spark's percentile to calculate the exact percentile rather than using the existing hasApproxQuantile check with error = 0.0. I assume this is because the latter uses Spark's approx quantile method.

When merging, please use the Squash and merge option.

import com.amazon.deequ.analyzers.Preconditions.{hasColumn, isNumeric}
import com.amazon.deequ.analyzers.Analyzers.{conditionalSelection, ifNoNullsIn}
import com.amazon.deequ.metrics.FullColumn
import org.apache.curator.shaded.com.google.common.annotations.VisibleForTesting
Copy link
Contributor

Choose a reason for hiding this comment

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

We use import com.google.common.annotations.VisibleForTesting in other places.

@rdsharma26 rdsharma26 merged commit cedcf07 into awslabs:master Oct 27, 2023
1 check passed
rdsharma26 pushed a commit that referenced this pull request Oct 27, 2023
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
javierdlrm pushed a commit to javierdlrm/deequ that referenced this pull request Oct 31, 2023
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
rdsharma26 pushed a commit that referenced this pull request Nov 1, 2023
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
rdsharma26 pushed a commit that referenced this pull request Apr 16, 2024
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
rdsharma26 pushed a commit that referenced this pull request Apr 16, 2024
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
rdsharma26 pushed a commit that referenced this pull request Apr 16, 2024
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
rdsharma26 pushed a commit that referenced this pull request Apr 17, 2024
* Creation of Exact Quantile Check

* Fix build issue

---------

Co-authored-by: ZDQ870 <[email protected]>
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.

2 participants