-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[WIP] New Feature: Add AverageRegressor #10868
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
[WIP] New Feature: Add AverageRegressor #10868
Conversation
' tuples') | ||
|
||
# validate that at least one estimator is not None | ||
isnone = [clf is not None for _, clf in self.estimators] |
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.
Can't we avoid duplication with respect to VotingClassifier?
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.
We should also avoid duplicating tests for voting classifier. Can we put them in the same module, e.g. ensemble/voting.py
@jnothman Thanks for reviewing, I refactored the code to |
is this still wip?
|
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.
You're still duplicating lots of tests, aren't you? Can you just parameterize some tests with each of VotingClassifier and AverageRegressor?
@jnothman it's still WIP because I haven't finished the examples and the documentation update. Also, I am still not fully happy with the state of the code right now, I need to reduce the amount of duplicated code as much as possible before switching to MRG. But If you have any other suggestion or comments, that would be helpful. Thanks |
Do you plan to complete this PR? |
@stsouko yes, I'll spare few hours this week to finish it. |
Closed by #12513 |
Reference Issues/PRs
Fixes #10743
What does this implement/fix? Explain your changes.
Following @amueller suggestion about adding
sklearn.ensemble.AverageRegressor
, this PR implements:Any other comments?
This PR is inspired from work on
bagging.py
,VotingClassifier
and their associated unit-tests.