-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC add link to plot_voting_probas.py for Voting Classifier in ensemble.rst #30847
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
Conversation
doc/modules/ensemble.rst
Outdated
.. rubric:: Examples | ||
|
||
* :ref:`sphx_glr_auto_examples_ensemble_plot_stack_predictors.py` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @elhambbi,
thank you, but it seems something went wrong here. You had already added this example in #30747, which was merged into main 6 days ago.
Please, before moving forward, update your main branch with
git fetch upstream
git rebase upstream/main
to pull the latest changes and then either merge these changes into your feature branch or (easier) create a new branch.
Please do then re-evaluate this PR and push again or (easier) make a new PR from your new branch.
Thank you for your efforts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HIi @StefanieSenger . Thanks for your feedback.
I merged the changes from the main into my branch and pushed again.
It should be ok now.
Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you a lot for your further work, @elhambbi!
I would have two suggestions that would lead to one reference not being added and the other one added but in another spot. I hope that is okay for you.
The link to plot_voting_probas.py
is a valuable addition, since it had not been linked anywhere before. Though I would think it should go in the section "1.11.4.3. Weighted Average Probabilities (Soft Voting)" where soft voting is treated, as in the example. And it would be nice if you can integrate this into the text, not in an examples section.
Be aware that if you add a link like this :ref:
title ``, you can change its title so that not the example's title is shown but whatever you pick. This way it can integrate more nicely into the sentences.
Do you think that would fit with you?
doc/modules/ensemble.rst
Outdated
@@ -1485,6 +1485,11 @@ Optionally, weights can be provided for the individual classifiers:: | |||
>>> grid = GridSearchCV(estimator=eclf, param_grid=params, cv=5) | |||
>>> grid = grid.fit(iris.data, iris.target) | |||
|
|||
.. rubric:: Examples | |||
|
|||
* :ref:`sphx_glr_auto_examples_ensemble_plot_voting_decision_regions.py` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* :ref:`sphx_glr_auto_examples_ensemble_plot_voting_decision_regions.py` |
I would rather leave this one out, since it is linked in line 1445 with an image and that is pretty close.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @StefanieSenger .
I removed reference to plot_voting_decision_regions.py
and added plot_voting_probas.py
within the related text :). Let me know if you prefer to modify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work, @elhambbi!
I have two nits, but these are both details. Feel free to ignore.
Do you think we can merge this one, @adrinjalali or @marenwestermann?
doc/modules/ensemble.rst
Outdated
@@ -1485,6 +1486,7 @@ Optionally, weights can be provided for the individual classifiers:: | |||
>>> grid = GridSearchCV(estimator=eclf, param_grid=params, cv=5) | |||
>>> grid = grid.fit(iris.data, iris.target) | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc/modules/ensemble.rst
Outdated
highest average probability. See :ref:`plot probabilities <sphx_glr_auto_examples_ensemble_plot_voting_probas.py>` for | ||
a detailed illustration of class probabilities averaged by soft voting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
highest average probability. See :ref:`plot probabilities <sphx_glr_auto_examples_ensemble_plot_voting_probas.py>` for | |
a detailed illustration of class probabilities averaged by soft voting. | |
highest average probability. See this example on | |
:ref:`Visualising class probabilities in a Voting Classifier <sphx_glr_auto_examples_ensemble_plot_voting_probas.py>` | |
for a detailed illustration of class probabilities averaged by soft voting. |
In the hope this more respects our max character length of 88 per line.
And removing "plot" from the title, which is more like an intern naming convention. Does that resonate with you? Feel free to change again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @StefanieSenger.
I made the changes to maintain consistency with ScikitLearn standards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for modifying this @elhambbi!
Hallo @elhambbi, I am sorry, but it seems that you have pushed the reference to this new example to the same branch. I've seen this happening if people use tools like VSCode to manage git, when they don't set that each new local branch should create a new remote branch and then accidentally they push to the wrong branch. If this is what happened, then I would recommend to learn some more git. (I really liked the Oh my git! game for learning git, but since you need to download it, let me add: no garantee that it's alright, I didn't check.) Let's link to different examples in separate branches. The reason for it is that it is easier for maintainers to review and merge. Can you please remove this new link and push again? |
Hi @StefanieSenger sorry for the conflict. |
Thank you @elhambbi! This now can be reviewed by a maintainer. @marenwestermann @adrinjalali, do you want to have a look? |
Reference Issues/PRs
Towards #30621
What does this implement/fix? Explain your changes.
The examples for Voting Classifier were missed.
I have added two examples
plot_voting_decision_regions.py
andplot_voting_probas.py
for Voting Classifier in ensemble.rstAny other comments?