Skip to content

DOC Merge plot_svm_margin.py and plot_separating_hyperplane.py into plot_svm_hyperplane_margin.py #31045

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

SwathiR1999
Copy link

@SwathiR1999 SwathiR1999 commented Mar 21, 2025

Reference Issues/PRs

References #30621

What does this implement/fix? Explain your changes.

  • Consolidated two SVM example scripts (plot_svm_margin.py and plot_separating_hyperplane.py) into a single file plot_svm_hyperplane_margin.py.
  • Updated explanation for clarity.

Any other comments?

This PR addresses the feedback from #30975, which was closed.

@StefanieSenger Looking forward to your feedback. Thanks!

Copy link

github-actions bot commented Mar 21, 2025

❌ Linting issues

This PR is introducing linting issues. Here's a summary of the issues. Note that you can avoid having linting issues by enabling pre-commit hooks. Instructions to enable them can be found here.

You can see the details of the linting issues under the lint job here


ruff format

ruff detected issues. Please run ruff format locally and push the changes. Here you can see the detected issues. Note that the installed ruff version is ruff=0.11.7.


--- examples/svm/plot_svm_hyperplane_margin.py
+++ examples/svm/plot_svm_hyperplane_margin.py
@@ -44,17 +44,16 @@
     misclassified = y_pred != y
 
     plt.subplot(1, 3, i)
-    plt.scatter(X[:, 0], X[:, 1], c=y, s=30,
-                cmap=plt.cm.Paired, edgecolors="k")
+    plt.scatter(X[:, 0], X[:, 1], c=y, s=30, cmap=plt.cm.Paired, edgecolors="k")
     # misclassified samples
     plt.scatter(
         X[misclassified, 0],
         X[misclassified, 1],
-        facecolors='none',
-        edgecolors='k',
+        facecolors="none",
+        edgecolors="k",
         s=80,
         linewidths=1.5,
-        label="Misclassified"
+        label="Misclassified",
     )
 
     # plot the decision function

1 file would be reformatted, 918 files already formatted

Generated for commit: 2ef1d5f. Link to the linter CI: here

@SwathiR1999 SwathiR1999 changed the title Doc Merge plot_svm_margin.py and plot_separating_hyperplane.py into plot_svm_hyperplane_margin.py DOC Merge plot_svm_margin.py and plot_separating_hyperplane.py into plot_svm_hyperplane_margin.py Mar 21, 2025
@StefanieSenger StefanieSenger self-requested a review April 28, 2025 11:26
Copy link
Contributor

@StefanieSenger StefanieSenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @SwathiR1999,

thank you for your patience and for your efforts to merge the examples. It looks very good and I like the design of the plots. The plots all have heading and legend and are easy on the eye.

I think we could add some information and I have left you some suggestions on that, too.

When I build the docs locally, I cannot click to open it and also there is some text overflow in the background. I believe this is because of a rending issue in the heading, and I have added a suggestions for a fix.

What is lacking is to remove the old examples and to add redirections from both old links to the new example link in the doc/conf.py file. You can have a look at #30028 to see how to add the redirections.

You need to delete all links to the old examples as well (and replace it with the new one if suitable). There are some, probably too many, links to plot_separating_hyperplane_unbalanced currently.

Comment on lines 10 to 15
- **Small C (e.g., 0.05)**:
- Allows some misclassifications, resulting in a wider margin.
- **Moderate C (e.g., 1)**:
- Balances classification accuracy and margin width.
- **Large C (e.g., 1000)**:
- Prioritizes classifying all points correctly, leading to a narrower margin.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think while reworking the example, we could add some more information.

Do you think you could add some suggestion of when to use a small, a moderate or a high C value in a practical use case?

We could also talk about overfitting and underfitting in this context.

Also, we could talk about miss-classification / error for different margins.

I think this could be added after the code for the plot using the # % syntax to make this example more notebook-like. (Also have a look at #30028 for a good example on the notebook style.)

"""
=========================================================================
SVM: Effect of Regularization (C) on Maximum Margin Separating Hyperplane
========================================================================
Copy link
Contributor

@StefanieSenger StefanieSenger Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
========================================================================
=========================================================================

The lacking "=" leads to the title not being rendered correctly. Adding "=" should fix it.

I also hope that this fixes the problem with the example not being clickable from the main examples page and the text overflow there (see other comment).

clf.fit(X, y)

plt.subplot(1, 3, i)
plt.scatter(X[:, 0], X[:, 1], c=y, s=30, cmap=plt.cm.Paired, edgecolors="k")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add axis labels. Something simple like "Feature 1" and "Feature 2" would be enough.

s=100,
linewidth=1.5,
facecolors="none",
edgecolors="r",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about having a different color for miss-classified samples?

@SwathiR1999
Copy link
Author

Hi @StefanieSenger , thanks for the feedback!

There are some, probably too many, links to plot_separating_hyperplane_unbalanced currently.

Just to clarify - should I also apply similar changes (merge/update/remove references) to plot_separating_hyperplane_unbalanced.py, or is the current focus only on replacing plot_svm_margin.py and plot_separating_hyperplane.py with the new plot_svm_hyperplane_margin.py example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants