[MRG] FIX OvR with constant label for non-predict methods #3308
Conversation
|
This is still broken for a constantly absent label. |
|
This fixes the problem in my case. |
|
Until the second commit, it may have fixed the error, but not given correct output. The second commit should resolve that. |
jnothman
changed the title
| @@ -63,10 +63,23 @@ def test_ovr_always_present(): | |||
| X[:5, :] = 0 | |||
| y = [[int(i >= 5), 2, 3] for i in range(10)] | |||
| with warnings.catch_warnings(record=True): | |||
| ovr = OneVsRestClassifier(DecisionTreeClassifier()) | |||
| ovr = OneVsRestClassifier(LogisticRegression()) | |||
There was a problem hiding this comment.
Why do you need to modify this line?
There was a problem hiding this comment.
Same question here. Maybe this is because the predict_proba of a DecisionTreeClassifier is pretty meaningless as it's binary unless the depth is bound. That should still work though.
There was a problem hiding this comment.
When I first tried, it seemed there was no predict_proba on the tree, but I thought that was a bit weird. I was doing something else at the same time....
There was a problem hiding this comment.
Right. With DTC, I get Base estimator doesn't have a decision_function attribute.
There was a problem hiding this comment.
Then it was some other class I threw in that refused to give me a predict_proba
There was a problem hiding this comment.
If SVC, why not LogisticRegression?
On 24 June 2014 09:37, Arnaud Joly notifications@github.com wrote:
In sklearn/tests/test_multiclass.py:
@@ -63,10 +63,23 @@ def test_ovr_always_present():
X[:5, :] = 0
y = [[int(i >= 5), 2, 3] for i in range(10)]
with warnings.catch_warnings(record=True):
ovr = OneVsRestClassifier(DecisionTreeClassifier())ovr = OneVsRestClassifier(LogisticRegression())svc?
—
Reply to this email directly or view it on GitHub
https://github.com/scikit-learn/scikit-learn/pull/3308/files#r14128965.
There was a problem hiding this comment.
+1 for LogisticRegression as the simplest classifier that models class assignment probabilities. Tied with MultinomialNB for event count data.
There was a problem hiding this comment.
The decision_function of MultinomialNB is kind of a hack (a linear interpretation of the model) so +2 for LogisticRegression.
|
Could you use assert_warns or ignore_warnings instead of the context manager? |
|
+1 for merging. |
|
Thanks @jnothman merging! |
[MRG] FIX OvR with constant label for non-predict methods
bug mentioned on ML