Skip to content

Added link to plot_grid_search_text_feature_extraction.py under TfidfVectorizer#30974

Merged
marenwestermann merged 4 commits intoscikit-learn:mainfrom
AnthonyPrudent:my_feature
Sep 17, 2025
Merged

Added link to plot_grid_search_text_feature_extraction.py under TfidfVectorizer#30974
marenwestermann merged 4 commits intoscikit-learn:mainfrom
AnthonyPrudent:my_feature

Conversation

@AnthonyPrudent
Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Towards #30621

What does this implement/fix? Explain your changes.

Added a link to plot_grid_search_text_feature_extraction.py in the API documentation under TfidfVectorizer.

Any other comments?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 11, 2025

✔️ Linting Passed

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

Generated for commit: a0cbb14. Link to the linter CI: here

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.

Thanks for your contribution, @AnthonyPrudent! It looks all good to me and I think it can be merged.

Apart from this PR, I am not so happy about the enumeration-like listing of links to examples in the TFIDF docstring, but that's a problem that was existing before.

Maybe a follow up task could be to mention these in the :ref:User Guide <text_feature_extraction> section of the User Guide instead of here. I would suggest to open an issue with a label for moderate difficulty for this, since this requires some more understanding of the topic.

What do you think, @adrinjalali?

@adrinjalali
Copy link
Copy Markdown
Member

Like the other example where we add a link to a specific section of the user guide instead, I think this makes sense here as well as @StefanieSenger suggests. So I'd suggest moving the list in a nicely iterated way to the user guide if it's not already there, and leave a link to that particular section here.

That is, unless one of the examples tackles a specific constructor argument where a link to the example in that parameter makes sense.

@marenwestermann
Copy link
Copy Markdown
Member

Hi @AnthonyPrudent! Would you like to continue working on this pull request?

@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

@lucyleeow lucyleeow reopened this Sep 8, 2025
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. If the rendered docs look good and if the CI is green, I will merge.

@marenwestermann marenwestermann merged commit c17bd23 into scikit-learn:main Sep 17, 2025
36 checks passed
@marenwestermann
Copy link
Copy Markdown
Member

Thank you for your work on this PR @AnthonyPrudent!

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