[MRG] scorer: add sample_weight support (+test) #3401
Conversation
|
The test is simple but the change is equally simple, just passing the |
|
|
||
| for name, scorer in SCORERS.items(): | ||
| try: | ||
| weighted = scorer(estimator[name], X_test, y_test, |
There was a problem hiding this comment.
I would rather have a more explicit test. For example having for the regressor [0, 0, 1, 1, 2, 2] as output and then using weights=[0, 0, 1, 1, 0, 0] and once 1-weights and check that the expected thing happens.
There was a problem hiding this comment.
@amueller The difficulty is that, unlike when testing the metrics directly, for testing the scorer it's tricky to anticipate the output of the classifier / regressor.
There was a problem hiding this comment.
not if you use the dummys ;) What is wrong with my suggestion?
There was a problem hiding this comment.
The 'expected thing' would need to be manually computed for each different scorer, right?
That would be just like coming up with a different hand-crafted test for each scorer, which I could do, if you insist.
There was a problem hiding this comment.
You are right, if you are testing all score_functions, that doesn't make
any sense.
On 07/16/2014 05:23 PM, Vlad Niculae wrote:
In sklearn/metrics/tests/test_score_objects.py:
- sample_weight = np.ones_like(y_test)
- sample_weight[:10] = 0
get sensible estimators for each metric
- sensible_regr = DummyRegressor(strategy='median')
- sensible_regr.fit(X_train, y_train)
- sensible_clf = DecisionTreeClassifier()
- sensible_clf.fit(X_train, y_train)
- estimator = dict([(name, sensible_regr)
for name in REGRESSION_SCORER_NAMES] +[(name, sensible_clf)for name in CLF_SCORER_NAMES])
- for name, scorer in SCORERS.items():
try:weighted = scorer(estimator[name], X_test, y_test,The 'expected thing' would need to be manually computed for each
different scorer, right?That would be just like coming up with a different hand-crafted test
for each scorer, which I could do, if you insist.—
Reply to this email directly or view it on GitHub
https://github.com/scikit-learn/scikit-learn/pull/3401/files#r15004137.
|
LGTM when travis is green. |
|
lgtm |
|
Thanks a lot for taking this over @vene! I've had no time to work on this lately. |
| ignored = scorer(estimator[name], X_test[10:], y_test[10:]) | ||
| unweighted = scorer(estimator[name], X_test, y_test) | ||
| assert_not_equal(weighted, unweighted, | ||
| "scorer {} behaves identically when called with " |
There was a problem hiding this comment.
support for Py2.6 means we can't use {}. Use {0}, {1}, {2}, ...
|
Not much to do to get that merged. |
|
Travis is happy ! merging |
[MRG] scorer: add sample_weight support (+test)
|
Thanks! 🍻 |
|
I'm glad to see this moving through. Thanks Noel and Vlad. I look forward On 20 July 2014 07:47, Noel Dawe notifications@github.com wrote:
|
Wraps up #3098 (a part of #1574), ready for review.
Initial description by @ndawe: