Skip to content

DOC remove plot_logistic.py example#30942

Merged
marenwestermann merged 7 commits intoscikit-learn:mainfrom
shbradki:doc-add-plot-logistic
Sep 8, 2025
Merged

DOC remove plot_logistic.py example#30942
marenwestermann merged 7 commits intoscikit-learn:mainfrom
shbradki:doc-add-plot-logistic

Conversation

@shbradki
Copy link
Copy Markdown
Contributor

@shbradki shbradki commented Mar 4, 2025

Reference Issues/PRs

Towards #30621

What does this implement/fix? Explain your changes.

  • Adds a missing reference to plot_logistic.py in the Logistic Regression section of the User Guide (linear_model.rst).

  • Updates the LogisticRegression docstring (_logistic.py) to include a reference to the example.

Any other comments?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 4, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

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

@StefanieSenger
Copy link
Copy Markdown
Member

Hello @shbradki,

thank you for your PR! The links seem well placed to me, but you have to take care of the linting issues (the lines are too long). Best is to install pre-commit as mentioned in the issue and commit again.

Copy link
Copy Markdown
Member

@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.

Thank you @shbradki, that's a valuable addition to the docs.

I only have a few nits.

shbradki and others added 2 commits March 5, 2025 19:15
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
@shbradki
Copy link
Copy Markdown
Contributor Author

shbradki commented Mar 5, 2025

Thank you @shbradki, that's a valuable addition to the docs.

I only have a few nits.

Thank you, I thought I had edited the rst file as well but evidently not. Thanks for the correction!

@StefanieSenger
Copy link
Copy Markdown
Member

Happy this PR moved so fast.

@adrinjalali, can we merge it?

@adrinjalali
Copy link
Copy Markdown
Member

@shbradki thank for the contribution.

Looking at the example, I find the example a bit odd. It's comparing a regression model (linear regression) with a classification model (logistic regression).

On top of that, we already have other, much better logistic regression examples.

So I think we can remove the example altogether, and redirect to one of the existing examples. You can check an example of how to do so here: #30906

@StefanieSenger
Copy link
Copy Markdown
Member

Hi @shbradki, would you want to work on what @adrinjalali had suggested?

@shbradki
Copy link
Copy Markdown
Contributor Author

Hi @adrinjalali @StefanieSenger

So sorry about the delayed response, I'm just seeing this. I appreciate the feedback and would be happy to work on that. I'll remove the reference to plot_logistic.py and update the reference to point to a more relevant logistic regression example. Will follow up shortly with a revised commit.

@StefanieSenger
Copy link
Copy Markdown
Member

Awesome, @shbradki! :)

@Gouravbirwaz

This comment was marked as spam.

@StefanieSenger
Copy link
Copy Markdown
Member

Hi, @Gouravbirwaz. @shbradki is already working on it. Feel free, for instance, to check out #30621 (or other beginner-friendly issues) to find a new task for yourself.

@lucyleeow
Copy link
Copy Markdown
Member

I am going to close this due to inactivity and because we have decided to close #30621.

@lucyleeow lucyleeow closed this Sep 8, 2025
@lucyleeow
Copy link
Copy Markdown
Member

Re-opening, as this was reviewed and someone else may wish to continue

Copy link
Copy Markdown
Member

@marenwestermann marenwestermann left a comment

Choose a reason for hiding this comment

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

I updated the PR and addressed the comments. I will merge when the CI is green. Thank you @shbradki for your work on this PR!

@marenwestermann marenwestermann changed the title DOC: Add missing link to plot_logistic.py in Logistic Regression documentation DOC remove plot_logistic.py example Sep 8, 2025
@marenwestermann marenwestermann merged commit ceafd58 into scikit-learn:main Sep 8, 2025
37 checks passed
@StefanieSenger
Copy link
Copy Markdown
Member

Thanks, @marenwestermann. You're the savior of so many abandoned PRs! 🥇

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.

6 participants