Sample weight sliced to work with cross validation, issue #358 #359
Conversation
Codecov Report
@@ Coverage Diff @@
## master #359 +/- ##
==========================================
+ Coverage 97.32% 97.32% +<.01%
==========================================
Files 49 49
Lines 3138 3142 +4
Branches 584 585 +1
==========================================
+ Hits 3054 3058 +4
Misses 44 44
Partials 40 40
|
|
Thanks for the suggestions @lopuhin, I've added them in the two last commits |
| if weights is None: | ||
| est = clone(self.estimator).fit(X[train], y[train], **fit_params) | ||
| else: | ||
| est = clone(self.estimator).fit(X[train], y[train], sample_weight=weights[train], **fit_params) |
There was a problem hiding this comment.
This does the right thing, but it's possible to improve this a bit - instead of repeating almost the same line twice, it's possible to modify fit_params or create new fold_fit_params, putting modified sample_weight there - then the same call can be done for both cases, avoiding repetition.
There was a problem hiding this comment.
have a look at the new changes and see if they make sense. Good suggestions
| @@ -214,8 +214,13 @@ def _cv_scores_importances(self, X, y, groups=None, **fit_params): | |||
| cv = check_cv(self.cv, y, is_classifier(self.estimator)) | |||
| feature_importances = [] # type: List | |||
| base_scores = [] # type: List[float] | |||
| weights = fit_params.get('sample_weight', None) | |||
| fit_params.pop('sample_weight', None) | |||
There was a problem hiding this comment.
An minor improvement is possible here: pop already returns what it popped, so these two lines could be reduced to one:
weights = fit_params.pop('sample_weight', None)
Changes that could fix situation described in issue #358