-
Notifications
You must be signed in to change notification settings - Fork 87
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
Implemented lower threshold and window size #3627
Implemented lower threshold and window size #3627
Conversation
…g with updated unit tests
Codecov Report
@@ Coverage Diff @@
## main #3627 +/- ##
=======================================
+ Coverage 99.7% 99.7% +0.1%
=======================================
Files 335 335
Lines 33508 33529 +21
=======================================
+ Hits 33386 33407 +21
Misses 122 122
Continue to review full report at Codecov.
|
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.
Looks great! Just a few comments on clean ups
evalml/tests/data_checks_tests/test_datetime_format_data_check.py
Outdated
Show resolved
Hide resolved
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!
evalml/tests/data_checks_tests/test_datetime_format_data_check.py
Outdated
Show resolved
Hide resolved
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!
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 a nit for clarity but not blocking
@@ -39,11 +39,11 @@ def validate(self, X, y): | |||
Examples: | |||
>>> import pandas as pd | |||
|
|||
The column "dates" has a set of dates with hourly frequency appended to the end of a series of days, which is inconsistent | |||
with the frequency of the previous 9 dates (1 day). | |||
The column "dates" has a set of two dates with monthly frequency appended to the end of a set of two dates with hourly frequency that is also appended to the end |
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.
nit, but it'd be clearer to say
"The column 'dates' has a set of two dates with daily frequency, two dates with hourly frequency, and two dates with monthly frequency" or something of that sort
…ning' of github.com:alteryx/evalml into TML-3639-ds-feedback-ts-bug-time-index-format-check-warning
Pull Request Description
Implemented lower threshold and window size along with corresponding unit tests