[WIP] New Feature: Add AverageRegressor#10868
[WIP] New Feature: Add AverageRegressor#10868mohamed-ali wants to merge 25 commits intoscikit-learn:masterfrom mohamed-ali:add-AverageRegressor
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.
Can't we avoid duplication with respect to VotingClassifier?
jnothman
left a comment
There was a problem hiding this comment.
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?
|
jnothman
left a comment
There was a problem hiding this comment.
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,VotingClassifierand their associated unit-tests.