Skip to content

DOC Add link to plot_tree_regression.py example #26962

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 5 commits into from
Oct 18, 2024

Conversation

Tialo
Copy link
Contributor

@Tialo Tialo commented Aug 1, 2023

Reference Issues/PRs

This pr adds link in DecisionTreeRegressor to example file tree/plot_tree_regression.py as mentioned in #26927

What does this implement/fix? Explain your changes.

Any other comments?

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

✔️ Linting Passed

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

Generated for commit: 59ff618. Link to the linter CI: here

@adrinjalali
Copy link
Member

I think plot_tree_regression.py and plot_tree_regression_multioutput.py could be merged together and improved, and only keep one of them. WDYT @glemaitre

@adrinjalali
Copy link
Member

I've merged the two examples here. WDYT @marenwestermann @glemaitre

@glemaitre glemaitre self-requested a review October 17, 2024 14:10
We can see that if the maximum depth of the tree (controlled by the
`max_depth` parameter) is set too high, the decision trees learn too fine
details of the training data and learn from the noise, i.e. they overfit.
========================
"""
Copy link
Member

Choose a reason for hiding this comment

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

I would still write a small intro of what we will see

# `max_depth` parameter) is set too high, the decision trees learn too fine
# details of the training data and learn from the noise, i.e. they overfit.
#
# Necessary Imports
Copy link
Member

Choose a reason for hiding this comment

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

Let's delay the import in the cell that we use them.

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.

It looks good. I would not propose any further improvement. I think that we could revisit this example in another PR with this intent.

Copy link
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.

LGTM. I'm happy with it as is but also happy to have a small intro and move the import statements. I don't have a strong preference here.

@adrinjalali adrinjalali enabled auto-merge (squash) October 18, 2024 06:04
@adrinjalali adrinjalali merged commit 325930e into scikit-learn:main Oct 18, 2024
28 checks passed
@Tialo Tialo deleted the docs/add-tree-regression-links branch November 5, 2024 10:58
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.

4 participants