-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG] New Feature: VotingRegressor #12513
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
tests done. this is fork of PR #10868 which is not being finalized for a long time.
flake fixed.
Please add the [WIP] tag. Remember add [MRG] when you finish. |
# Conflicts: # doc/whats_new/v0.21.rst
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.
LGTM. I read the codes and IMO its ok.
We have "Voting" classifier, but "Average" regressor. Should it be "AveragingRegressor" for consistency? |
* AverageRegressor renamed to AveragingRegressor. * user guide fixed. * tests fixed. additional changes removed. * docstrings fixed. * user guide example plot fixed.
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.
Tests need some work
test_notfitted extended. duplicates removed. DummyRegressor used. only for unique methods of AveragingRegressor tests added.
I removed duplicate tests which checks common code. |
@jnothman, tests fixed. VotingClassifier specific tests I can fix in new PR. |
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.
First round of comments, looks good in general
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.
First round of comments, looks good in general
# Conflicts: # doc/whats_new/v0.21.rst
Co-Authored-By: stsouko <stsouko@live.ru>
Co-Authored-By: stsouko <stsouko@live.ru>
Co-Authored-By: stsouko <stsouko@live.ru>
Co-Authored-By: stsouko <stsouko@live.ru>
Co-Authored-By: stsouko <stsouko@live.ru>
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.
Last comments, LGTM otherwise
Co-Authored-By: stsouko <stsouko@live.ru>
Thanks @stsouko!! |
Thanks @stsouko! |
Thanks for the help in preparing PR |
This reverts commit 4f137ed.
This reverts commit 4f137ed.
Reference Issues/PRs
Fixes #10743
What does this implement/fix? Explain your changes.
Following @amueller suggestion about adding sklearn.ensemble.AverageRegressor, this PR implements:
sklearn.ensemble.AverageRegressor
Associated units tests
Docstrings
Examples
Updates the documentation
Updates what's new
Any other comments?
This PR is inspired from work on
VotingClassifier
and their associated unit-tests.this is fork of PR #10868 which is not being finalized for a long time.