-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC Add links to pipelines
examples in docstrings
#26904
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
DOC Add links to pipelines
examples in docstrings
#26904
Conversation
❌ Linting issuesThis PR is introducing linting issues. Here's a summary of the issues. Note that you can avoid having linting issues by enabling You can see the details of the linting issues under the
|
Could you please add in this issue (#26927) a comment with a link to the PR for each PR of this type and the examples they're adding to the docs? |
sklearn/pipeline.py
Outdated
@@ -69,6 +69,11 @@ class Pipeline(_BaseComposition): | |||
to another estimator, or a transformer removed by setting it to | |||
`'passthrough'` or `None`. | |||
|
|||
For examples use cases of `Pipeline` combined with |
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.
For examples use cases of `Pipeline` combined with | |
For use case examples of :class:`Pipeline` combined with |
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.
Also here, the link would link to the currently open document.
sklearn/pipeline.py
Outdated
@@ -69,6 +69,11 @@ class Pipeline(_BaseComposition): | |||
to another estimator, or a transformer removed by setting it to | |||
`'passthrough'` or `None`. | |||
|
|||
For examples use cases of `Pipeline` combined with |
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.
I would split this into two sentences.
The first sentence gives an example of the simplest pipeline use-case: pipelining estimators (PCA + learner). The second sentence illustrates how to optimize the hyperparameters of estimators within a pipeline.
I think the second example could be added directly in the discussion above that explains the __
story because it is precisely what you need to know for the GridSearchCV
.
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, I have separates those. Though I left the second sentence below the first one instead of pulling it up, because it is indeed the more complex example.
Updating with |
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.
In other PRs, we've moved the cases where it's mostly just "For a more detailed example see ..." to the "Examples" section. Otherwise LGTM.
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Thanks @adrinjalali, I have evaluated the positions of the 0815 example links and moved some to the "Examples" section. |
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. The linter issue is fixed if you merge with main.
Thanks @StefanieSenger LGTM on my side. |
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
This PR suggests to add links to the examples from the Pipelines and composite estimators section in the examples to the user guide and to docstrings of the respective classes and functions.