Skip to content
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

[MRG] DOC Add dropdown to Module 6.1 Pipelines and composite estimators #27022

Merged
merged 14 commits into from
Sep 1, 2023

Conversation

raph333
Copy link
Contributor

@raph333 raph333 commented Aug 6, 2023

Reference Issues/PRs

Add dropdowns to submodule 6.1. Pipelines and composite estimators regarding #26617

What does this implement/fix? Explain your changes.

  • Add dropdown for section 'Warning :Side effect of caching transformers'

Seems like this section is only of interest to users who are already familiar with the basics pipelines and transformers. As such, it could be good to hide it with a dropdown to reduce the wall of text new users have to scroll through.

Any other comments?

Didn't see any other sections that would be obvious candidates for hiding in a dropdown. But happy to hear feedback and suggestions!

@github-actions
Copy link

github-actions bot commented Aug 6, 2023

✔️ Linting Passed

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

Generated for commit: 939e7c9. Link to the linter CI: here

@raph333 raph333 changed the title DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26647 DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26617 Aug 6, 2023
@raph333 raph333 changed the title DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26617 [WIP] DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26617 Aug 6, 2023
@raph333 raph333 changed the title [WIP] DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26617 [MRG] DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26617 Aug 6, 2023
@ArturoAmorQ
Copy link
Member

Thanks for the PR @raph333! I think I would also hide the subsections "Construction" "Accessing steps" and "Nested parameters" in 6.1.1.1 as they are user specific for beginners.

And now that you are working on this Module, can you also please change the section 6.1.1.2. Notes to use instead a .. notes:: directive?

@raph333
Copy link
Contributor Author

raph333 commented Aug 7, 2023

Thank you for the review @ArturoAmorQ !
I updated the PR with the suggested changes. (see rendered docs here)

Comment on lines 177 to 191
-----
.. note::

Calling ``fit`` on the pipeline is the same as calling ``fit`` on
each estimator in turn, ``transform`` the input and pass it on to the next step.
The pipeline has all the methods that the last estimator in the pipeline has,
i.e. if the last estimator is a classifier, the :class:`Pipeline` can be used
as a classifier. If the last estimator is a transformer, again, so is the
pipeline.
Calling ``fit`` on the pipeline is the same as calling ``fit`` on
each estimator in turn, ``transform`` the input and pass it on to the next step.
The pipeline has all the methods that the last estimator in the pipeline has,
i.e. if the last estimator is a classifier, the :class:`Pipeline` can be used
as a classifier. If the last estimator is a transformer, again, so is the
pipeline.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please move this note to the end of Section 6.1.1? Right after the paragraph

All estimators in a pipeline, except the last one ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please move this note to the end of Section 6.1.1? Right after the paragraph

All estimators in a pipeline, except the last one ...

Sure, that makes sense @ArturoAmorQ!

I moved the notes and updated the PR (see rendered docs here).

Cheers!

@ArturoAmorQ ArturoAmorQ changed the title [MRG] DOC Add dropdown to Module 6.1 'Pipelines and composite estimators' #26617 [MRG] DOC Add dropdown to Module 6.1 Pipelines and composite estimators Aug 28, 2023
Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your work @raph333, merging!

@ArturoAmorQ ArturoAmorQ merged commit 4f17b5a into scikit-learn:main Sep 1, 2023
27 checks passed
@raph333
Copy link
Contributor Author

raph333 commented Sep 1, 2023

LGTM. Thanks for your work @raph333, merging!

Thanks for the feedback & review!

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.

None yet

2 participants