-
Notifications
You must be signed in to change notification settings - Fork 69
fix: Raise an error when CrossValidationReport
fails before at least one estimator is fitted
#1574
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
base: main
Are you sure you want to change the base?
Conversation
…efore at least one estimator is fitted, raise an error probabl-ai#1561
CrossValidationReport
fails before at least one estimator is fitted
Coverage Report for backend
|
Co-authored-by: Guillaume Lemaitre <[email protected]>
Coverage Report for backend
|
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.
Just a nitpick, otherwise LGTM. Thanks.
Edit: can you please fix the merge-conflict?
if len(estimator_reports) == 0: | ||
raise RuntimeError( | ||
"Cross-validation failed: no estimators were successfully fitted. " | ||
"Please check your data, estimator, or cross-validation setup." | ||
) from e |
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.
Nitpick: can you move this block before the construction of the message (between 201 and 203)? It should be more readable.
Fixed #1561
I added an if condition at the end of the "crossvaldiationreport" funcrion to check if there are 0 estimators and raise an error.
I also added a test for it in the file "testcrossvalidation.py" where i created a broken estimator to test the case where no estimators are fitted