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

Fixing flaky TargetDistributionDataCheck test #2748

Merged
merged 2 commits into from Sep 7, 2021
Merged

Conversation

ParthivNaresh
Copy link
Contributor

@ParthivNaresh ParthivNaresh commented Sep 7, 2021

Fixes #2581

New test rounds the Shapiro value to the tenths place. Confirmed that the old test ran into a rounding issue once every few thousand runs. This test passed after 30,000 runs so I'm assuming the problem should be fixed.

This fix is fine for now until we can address #2749

@codecov
Copy link

codecov bot commented Sep 7, 2021

Codecov Report

Merging #2748 (d50f9fb) into main (214be16) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #2748   +/-   ##
=====================================
  Coverage   99.9%   99.9%           
=====================================
  Files        301     301           
  Lines      27819   27819           
=====================================
  Hits       27770   27770           
  Misses        49      49           
Impacted Files Coverage Δ
...alml/data_checks/target_distribution_data_check.py 100.0% <ø> (ø)
...hecks_tests/test_target_distribution_data_check.py 100.0% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 214be16...d50f9fb. Read the comment docs.

@ParthivNaresh ParthivNaresh changed the title Initial commit Fixing flaky TargetDistributionDataCheck test Sep 7, 2021
Copy link
Collaborator

@chukarsten chukarsten left a comment

Choose a reason for hiding this comment

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

Nice, thanks Parthiv!

@ParthivNaresh ParthivNaresh merged commit 9c8dd1d into main Sep 7, 2021
@ParthivNaresh ParthivNaresh deleted the lognormal_fix branch September 7, 2021 13:56
@chukarsten chukarsten mentioned this pull request Sep 10, 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.

Flaky test in nightlies: target distribution data check
2 participants