-
Notifications
You must be signed in to change notification settings - Fork 84
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
Update outliers data check implementation #1855
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1855 +/- ##
=========================================
+ Coverage 100.0% 100.0% +0.1%
=========================================
Files 260 260
Lines 20838 20892 +54
=========================================
+ Hits 20832 20886 +54
Misses 6 6
Continue to review full report at Codecov.
|
@@ -86,3 +86,39 @@ def test_outliers_data_check_string_cols(): | |||
details={"columns": ["d"]}).to_dict()], | |||
"errors": [] | |||
} | |||
|
|||
|
|||
def test_outlier_score(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private but added some tests anyways for sanity sake / code coverage :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin Looks great! I have some small comments about cleaning up the apis of the private static methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, way to knock out this initially poorly defined PR ;). I think we might be able to remove the nan check stuff, but I'll leave that up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Left one question, but nothing blocking
Closes #1745