8000 DOC use notebook-style in ensemble/plot_adaboost_regression.py by svenstehle · Pull Request #23263 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC use notebook-style in ensemble/plot_adaboost_regression.py #23263

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
May 16, 2022
Merged

DOC use notebook-style in ensemble/plot_adaboost_regression.py #23263

merged 10 commits into from
May 16, 2022

Conversation

svenstehle
Copy link
Contributor
@svenstehle svenstehle commented May 2, 2022 •
edited
Loading

Reference Issues/PRs

Updates ensemble/plot_adaboost_regression.py
For Issue #22406

What does this implement/fix? Explain your changes.

Updated the example plot_adaboost_regression.py to notebook style.

  • Changed imports to appear where first used
  • Heading underlinings are the length of the heading characters used
  • Changed order of plot, and changed to colorblind colors. Had to reshape array to deal with matplotlib warning
  • Changed citation to exchange the 8 with the correct H in H Drucker. Had to drop the . though. It was the culprit for some reason
  • Kept authors in first cell
  • Added some text in Train and predict with AdaBoost and DecisionTree Regressors. Happy to reword this though, just a first draft
  • Updated more wordings in heading and paragraph text to make the example more readable and well understood.

Any other comments?

Happy to receive feedback and implement improvements.

@svenstehle
Copy link
Contributor Author

This is how the current plot with colorblind colors looks like:
image

cmarmo
cmarmo approved these changes May 10, 2022
View reviewed changes
Copy link
Contributor
@cmarmo cmarmo left a comment

Choose a reason for hiding this comment

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

Thanks @svenstehle for your pull request.
Minor comment, otherwise looks good to me.

examples/ensemble/plot_adaboost_regression.py Outdated
@@ -9,26 +9,38 @@
regressor. As the number of boosts is increased the regressor can fit more
detail.

.. [1] H. Drucker, "Improving Regressors using Boosting Techniques", 1997.
.. [1] H Drucker, "Improving Regressors using Boosting Techniques", 1997.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind adding the link to the proceeding?
http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.21.5683&rep=rep1&type=pdf
I could not find the DOI (perhaps the article is still too old)... if by any chance you can find it, you might want to use the :doi: directive for the link.

@lesteve lesteve added the Quick Review For PRs that are quick to review label May 11, 2022
@glemaitre glemaitre self-assigned this May 13, 2022
glemaitre
glemaitre approved these changes May 13, 2022
View reviewed changes
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 pushed the small nitpicks proposed by @cmarmo and we can merge (just small last check on the rendering).

@svenstehle
Copy link
Contributor Author

Thanks for your reviews and the edit @cmarmo and @glemaitre :)
The linking of the doi looks nice. Incidentally, this also fixes the H. Drucker problem. Now the H. works again:
image

I took the liberty of pushing that tiny change, hope that's alright

@cmarmo
Copy link
Contributor
cmarmo commented May 13, 2022

@glemaitre the doi is not correctly resolved. It points to another article about SVM optimization.

@svenstehle
Copy link
Contributor Author

You are correct @cmarmo. I am a bit embarrassed that I did not see it. Good of you to double-check this! :D
I am trying to find the correct doi for this. So far without success.

@cmarmo
Copy link
Contributor
cmarmo commented May 13, 2022

Thanks @svenstehle ! If you can't find the DOI perhaps the bare link will be enough. Thanks.

@svenstehle
Copy link
Contributor Author
svenstehle commented May 13, 2022 •
edited
Loading

I tried a few approaches to use the DOI, each one failed. DOI never gets resolved.

Thanks @svenstehle ! If you can't find the DOI perhaps the bare link will be enough. Thanks.

I managed to introduce the link to the pdf though. Sphinx syntax is weird. Even though that approach seems a bit brittle to me. Do you prefer linking to the summary page? I am good with it either way.

@cmarmo
Copy link
Contributor
cmarmo commented May 13, 2022

I managed to introduce the link to the pdf though. Sphinx syntax is weird. Even though that approach seems a bit brittle to me.

mmhhh.. You are right...

Do you prefer linking to the summary page? I am good with it either way.

The link to the summary is probably better as long as the link for the download is in the page. Thanks for your patience!

@svenstehle
Copy link
Contributor Author

Sure thing :) I updated the link to point to the summary page.

@glemaitre glemaitre merged commit 9c12fb0 into scikit-learn:main May 16, 2022
@glemaitre
Copy link
Member

Thanks @svenstehle It looks good. Merging.

@svenstehle svenstehle deleted the doc_ensemble_plot_adaboost_regression branch May 16, 2022 19:01
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request May 19, 2022
@svenstehle @glemaitre
DOC use notebook-style in ensemble/plot_adaboost_regression.py (sciki…
08b2f0e 
…t-learn#23263)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
glemaitre added a commit that referenced this pull request May 19, 2022
@svenstehle @glemaitre
DOC use notebook-style in ensemble/plot_adaboost_regression.py (#23263)
68cd3b7 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
lesteve pushed a commit to lesteve/scikit-learn that referenced this pull request May 19, 2022
@svenstehle @glemaitre @lesteve
DOC use notebook-style in ensemble/plot_adaboost_regression.py (sciki…
f598465 
…t-learn#23263)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmarmo cmarmo cmarmo approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees

@glemaitre glemaitre

Labels
Documentation Quick Review For PRs that are quick to review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@svenstehle @cmarmo @glemaitre @lesteve
0