FIX make sure _PassthroughScorer works with meta-estimators #31898
Conversation
There was a problem hiding this comment.
Thanks for the fix and the explanations yesterday, @adrinjalali!
I have looked through the PR and left some suggestions.
| except AttributeError: | ||
| pass | ||
|
|
||
| self._metadata_request = requests |
There was a problem hiding this comment.
If _PassthroughScorer doesn't act as a _MetadataRequester anymore, then we don't need it to inherit from it. It then also doesn't need to overwrite its set_score_request method and the tests to test the new error message are not needed either.
There was a problem hiding this comment.
I'll have a look and see if this would break things.
There was a problem hiding this comment.
I have tried it. It seems to all work.
| This uses `self._get_scorer_instance()` implemented in child objects to get the | ||
| raw scorer instance of the estimator, which will be ignored if `self.scoring` is | ||
| not None. |
There was a problem hiding this comment.
Is there a more straightforward way to call get_scorer("r2") and get_scorer("accuracy") ? I wonder why we jump to the child object's _get_scorer_instance only to refer to what is defined in RegressorMixin and ClassifierMixin afterwards. Can this directly be defined in RegressorMixin and ClassifierMixin?
There was a problem hiding this comment.
There's no way for us right now to access the scorer from those mixins. I thought of adding it there, but then it would only be used here. So not sure it's worth it to be added there.
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
Co-authored-by: Stefanie Senger <91849487+StefanieSenger@users.noreply.github.com>
Fixes #30937
The issue was that
_PassthroughScorertries to be smart with metadata-routing info it stores, which only works with simply consumers and not meta estimators (or rather, works when given aMetadataRequestbut notMetadataRouter).This fixes the issue, and improves the implementation in
RidgeCVwhich was arguably not perfect.cc @StefanieSenger @ogrisel @antoinebaker