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
Fixes UnboundLocalError: local variable 'cv_pipeline' referenced before assignment when error in automl search #996
Conversation
Codecov Report
@@ Coverage Diff @@
## main #996 +/- ##
==========================================
+ Coverage 99.67% 99.86% +0.19%
==========================================
Files 179 179
Lines 9424 9436 +12
==========================================
+ Hits 9393 9423 +30
+ Misses 31 13 -18
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.
@angela97lin Looks good to me! I left a comment about how to write a unit test for this. Apart from that, this made it clear to me that we don't have a good way of catching errors that come from splitting the data. That might be intended but if not we should discuss what to do about that.
@freddyaboulton Agreed, we don't have a clear way of telling the user that the error stemmed from splitting data, but with this in place, at least we'll default to our catching / logging of pipeline errors :D |
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 great reproducer on the issue, and your explanation of the problem on here was helpful! This rocks. I left one comment about checking for nan
scores in the test.
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 -agree with Dylan's comment on checking for nan
Closes #995
Before this PR, in the unlikely chance that
_compute_cv_scores
errors before settingcv_pipeline = pipeline.clone()
, then we get anUnboundLocalError: local variable 'cv_pipeline' referenced before assignment
in the later checkif isinstance(cv_pipeline, BinaryClassificationPipeline) and cv_pipeline.threshold is not None
. This PR addresses this by initializing it toNone
first before thetry
/except
block.`