-
Notifications
You must be signed in to change notification settings - Fork 1
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
improve error messages #77
Conversation
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 good! yeah, we probably should've kept the checks more tight.
(benchmark 5853999107 / attempt 1)
|
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 good, thanks.
Can we (in a separate PR) relax the condition to a warning and leave it to sklearn to raise an error if necessary?
Since we directly access every element of the state after the assertion, a warning wouldn't gain us anything since there would then be another error directly afterwards. |
#74 (comment)