Skip to content

DOC Update estimator representation example #24439

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

Merged
merged 2 commits into from
Sep 16, 2022

Conversation

betatim
Copy link
Member

@betatim betatim commented Sep 14, 2022

Reference Issues/PRs

Closes #21289

What does this implement/fix? Explain your changes.

This updates the example that showed off the "compact" representation of estimators. The compact representation is now the default and the HTML representation is less well known. This updates the example to reflect the new default and gives the HTML some more visibility.

Any other comments?

What should we call this example? Once we have a good name/title I will update the filename to reflect that.

@Deke1604

This comment was marked as spam.

@glemaitre
Copy link
Member

We need as well to redirect the previous example to the new one by editing conf.py: https://github.com/scikit-learn/scikit-learn/blob/main/doc/conf.py#L261

# In notebooks estimators and pipelines will use a rich HTML representation.
# This is particularly useful to summarise the
# structure of pipelines and other composite estimators, with interactivity to
# provide detail. Click on the example image below to expand Pipeline
Copy link
Member

Choose a reason for hiding this comment

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

It might be worth mentioning clicking on the "horizontal arrow" instead of the image.

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot 2022-09-16 at 11 54 46

This is what the image looks like for me, you can click nearly anywhere on that image and something will happen. So I'd vote for leaving the text as is. In addition I took the text nearly verbatim from the "release highlights" section, so I guess someone already thought about this/if it was good enough then it probably is good enough now as well.

WDYT?

Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

I like the example. Thanks @betatim LGTM

The compact representation is now the default and the HTML
representation is less well known. This updates the example to reflect
the new default and gives the HTML some more visibility.
@glemaitre glemaitre merged commit 268d6b4 into scikit-learn:main Sep 16, 2022
@glemaitre
Copy link
Member

OK Good enough then. Thanks @betatim Merging

@betatim betatim deleted the fancy-estimators branch September 18, 2022 05:52
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.

Add a demo of the HTML repr to "Compact estimator representations" example
3 participants