Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork26k
ENH: Add xlim/ylim parameters to DecisionBoundaryDisplay.from_estimator#31693
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 ourterms of service andprivacy 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?
ENH: Add xlim/ylim parameters to DecisionBoundaryDisplay.from_estimator#31693
Conversation
github-actionsbot commentedJul 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Thanks for your contribution!
Could you please add some tests and a whats new entry:https://github.com/scikit-learn/scikit-learn/blob/main/doc/whats_new/upcoming_changes/README.md
Uh oh!
There was an error while loading.Please reload this page.
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.
Let's also specify in the docstring thateps
is ignored if bothxlim
andylim
given. Could we also move the the new kwargs to underneatheps
?
Thanks for the swift review. I will add these changes soon. |
Hi@lucyleeow |
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.
Please place your tests in the existing decision boundary test:sklearn/inspection/_plot/tests/test_boundary_decision_display.py
.
You will be able to use the test data and fixtures, so you don't have to repeat.
Your test cases are probably too long and more than what is actually needed. If you are using LLMs to help you with tests, please note that they tend to produce verbose code and careful oversight and review by yourself is probably best.
Note also that we don't use test classes. I am happy to take another lookafter you've carefully reviewed your tests. Thank you
doc/whats_new/upcoming_changes/sklearn.inspection/31693.enhancement.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…nto feature/decision-boundary-xlim-ylim
ShauryaDusht commentedJul 7, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I’ve moved the tests to Also removed test classes and refactored the cases to be more generalized Thank you |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Reference Issues/PRs
Towards#27462
What does this implement/fix? Explain your changes.
This PR adds
xlim
andylim
parameters toDecisionBoundaryDisplay.from_estimator()
to allow users to override the automatic data-based plot limits.xlim=None
andylim=None
parameters to method signature