Skip to content

FIX metadata routing for scorer #31654

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

tomMoral
Copy link
Contributor

@tomMoral tomMoral commented Jun 25, 2025

Fixes #27977

This PR add metadata routing to the estimator to score when calling a scorer.
One thing that is not easy to understand is that here, the routing needs to be dynamic, as it depends on the estimator that is being scored.
For now it is implemented for unique scorer, I have to see how to do this for multiple scorer...

TODO

  • Add a test
  • Check that this works for multiple scorer

Copy link

github-actions bot commented Jun 25, 2025

❌ Linting issues

This PR is introducing linting issues. Here's a summary of the issues. Note that you can avoid having linting issues by enabling pre-commit hooks. Instructions to enable them can be found here.

You can see the details of the linting issues under the lint job here


ruff check

ruff detected issues. Please run ruff check --fix --output-format=full locally, fix the remaining issues, and push the changes. Here you can see the detected issues. Note that the installed ruff version is ruff=0.11.7.


sklearn/metrics/_scorer.py:41:5: F401 [*] `..utils.metadata_routing.process_routing` imported but unused
   |
39 |     _routing_enabled,
40 |     get_routing_for_object,
41 |     process_routing,
   |     ^^^^^^^^^^^^^^^ F401
42 | )
43 | from ..utils.validation import _check_response_method
   |
   = help: Remove unused import: `..utils.metadata_routing.process_routing`

sklearn/metrics/_scorer.py:616:89: E501 Line too long (91 > 88)
    |
614 |         return Bunch(
615 |             score={
616 |                 k: kwargs[k] for k in self.get_metadata_routing().consumes("score", kwargs)
    |                                                                                         ^^^ E501
617 |             }
618 |         )
    |

Found 2 errors.
[*] 1 fixable with the `--fix` option.

ruff format

ruff detected issues. Please run ruff format locally and push the changes. Here you can see the detected issues. Note that the installed ruff version is ruff=0.11.7.


--- sklearn/metrics/_scorer.py
+++ sklearn/metrics/_scorer.py
@@ -551,9 +551,7 @@
 
         self._metadata_request = MetadataRequest(owner=self.__class__.__name__)
         estimator_request = getattr(
-            estimator,
-            "_metadata_request",
-            estimator._get_default_requests()
+            estimator, "_metadata_request", estimator._get_default_requests()
         )
         if hasattr(estimator_request, "score"):
             self._metadata_request.score = copy.deepcopy(estimator_request.score)
@@ -613,7 +611,8 @@
     def _route_scorer_metadata(self, estimator, kwargs):
         return Bunch(
             score={
-                k: kwargs[k] for k in self.get_metadata_routing().consumes("score", kwargs)
+                k: kwargs[k]
+                for k in self.get_metadata_routing().consumes("score", kwargs)
             }
         )
 

--- sklearn/model_selection/_validation.py
+++ sklearn/model_selection/_validation.py
@@ -363,16 +363,20 @@
                 estimator=estimator,
                 # TODO(SLEP6): also pass metadata to the predict method for
                 # scoring?
-                method_mapping=MethodMapping().add(caller="fit", callee="fit")
+                method_mapping=MethodMapping().add(caller="fit", callee="fit"),
             )
         )
         try:
             routed_params = process_routing(router, "fit", **params)
             scorer_router = scorers._route_scorer_metadata(estimator, params)
-            routed_params.scorer = Bunch(score={
-                k: v for step in scorer_router.values()
-                for params in step.values() for k, v in params.items()
-            })
+            routed_params.scorer = Bunch(
+                score={
+                    k: v
+                    for step in scorer_router.values()
+                    for params in step.values()
+                    for k, v in params.items()
+                }
+            )
         except UnsetMetadataPassedError as e:
             # The default exception would mention `fit` since in the above
             # `process_routing` code, we pass `fit` as the caller. However,

2 files would be reformatted, 922 files already formatted

Generated for commit: db99d2c. Link to the linter CI: here

@tomMoral
Copy link
Contributor Author

A bit fighting with the sample_weight, which is a complicated edge case 😓
I think better documenting what the expected behavior is would help.

@glemaitre glemaitre self-requested a review June 26, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Routing metadata to the response_method used by a scorer
1 participant