Skip to content

DOC Add link to plot_tree_regression.py example#26962

Merged
adrinjalali merged 5 commits intoscikit-learn:mainfrom
Tialo:docs/add-tree-regression-links
Oct 18, 2024
Merged

DOC Add link to plot_tree_regression.py example#26962
adrinjalali merged 5 commits intoscikit-learn:mainfrom
Tialo:docs/add-tree-regression-links

Conversation

@Tialo
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Member

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

@glemaitre glemaitre self-requested a review October 17, 2024 14:10
`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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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.

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
@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