-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG] BUG Fixes duck typing in voting #14287
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
[MRG] BUG Fixes duck typing in voting #14287
Conversation
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.
CI is not happy.
('lr', clf1), ('rf', clf2), ('gnb', clf3)], | ||
voting='hard') | ||
eclf.fit(X, y) | ||
assert not hasattr(eclf, "predict_proba") |
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.
This would apply even before fit
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.
Otherwise lgtm
with pytest.raises(AttributeError, match=msg): | ||
eclf.predict_proba | ||
|
||
with pytest.raises(AttributeError, match=msg): |
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.
This is redundant wrt the previous assertion
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.
Thanks! I imagine we don't need a what's new entry for this?
A what's new entry doesn't hurt, and helps us to find things when problems
arise. "The predict_proba attribute will no longer be present when
voting='hard'."
|
Fixes the ducktyping of
predict_proba
inVotingClassifier
.