-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
FIX Array validation for DecisionBoundaryDisplay #25077
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
FIX Array validation for DecisionBoundaryDisplay #25077
Conversation
sklearn/inspection/_plot/tests/test_boundary_decision_display.py
Outdated
Show resolved
Hide resolved
glemaitre
left a comment
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.
Otherwise LGTM.
Co-authored-by: Guillaume Lemaitre <[email protected]>
thomasjpfan
left a comment
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.
Thank you for the PR @ArturoAmorQ !
sklearn/inspection/_plot/tests/test_boundary_decision_display.py
Outdated
Show resolved
Hide resolved
|
Actually, I would consider this as a fix and we should also add an entry in the changelog. |
jeremiedbb
left a comment
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. Thanks @ArturoAmorQ
|
Any idea of the reason for the Windows install fail? |
|
It's failing everywhere, but we did not find out why yet |
Co-authored-by: Guillaume Lemaitre <[email protected]>
Reference Issues/PRs
What does this implement/fix? Explain your changes.
DecisionBoundaryDisplayexpects the input dataXto be of shape (n_samples, 2), but this is not ensured by an internal validation. Because of this, the following code will output an unintuitiveValueErrormessage:Even worse, a further
makes a plot!
This PR is a simple fix to solve the issue.
Any other comments?
A meta comment: I am wondering if we could add a parameter
column_values(or similar) that accepts dictionaries to fix values forn_features - 2of the variables, allowing then for surface curves of the decision boundary display.