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
Hide LightGBM warnings #1342
Hide LightGBM warnings #1342
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1342 +/- ##
==========================================
+ Coverage 99.95% 99.95% +0.01%
==========================================
Files 213 213
Lines 13795 13814 +19
==========================================
+ Hits 13788 13807 +19
Misses 7 7
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.
@bchen1116 Looks great ! I left a couple of questions.
evalml/pipelines/components/estimators/classifiers/lightgbm_classifier.py
Outdated
Show resolved
Hide resolved
evalml/pipelines/components/estimators/classifiers/lightgbm_classifier.py
Outdated
Show resolved
Hide resolved
evalml/pipelines/components/estimators/classifiers/lightgbm_classifier.py
Outdated
Show resolved
Hide resolved
evalml/pipelines/components/estimators/classifiers/lightgbm_classifier.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.
@bchen1116 great! Two considerations:
-
Can we add unit test coverage which expects that there's no warning raised? For reference I think we do this in a few places in our tests, including in the elasticnet classifier tests
-
Were we seeing this warning in any usage of our lightgbm estimator? Or was it just for specific components? If its the latter, it would be great to have unit test coverage with a set of parameters which previously resulted in a warning.
I also left a comment on the impl
fix #1330