-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
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
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 <g.lemaitre58@gmail.com>
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. |
…nto DecisionBoundaryDisplay
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 <g.lemaitre58@gmail.com>
Reference Issues/PRs
What does this implement/fix? Explain your changes.
DecisionBoundaryDisplay
expects the input dataX
to be of shape (n_samples
, 2), but this is not ensured by an internal validation. Because of this, the following code will output an unintuitiveValueError
message: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 - 2
of the variables, allowing then for surface curves of the decision boundary display.