FEAT enable default routing strategy machinery #30946

adrinjalali · Mar 5, 2025

This creates the machinery needed to setup a default routing for metadata on the estimator level.

github-actions · Mar 5, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: fcbc431. Link to the linter CI: here}

adrinjalali · Mar 6, 2025

cc @antoinebaker @ogrisel @StefanieSenger

ogrisel · Mar 11, 2025

Thanks, @adrinjalali, for the PR. I opened a PR to this PR to actually define the default policy for sample_weight in BaseEstimator:

adrinjalali#5

Once we agree on this, and possibly on a little refactoring to also do it for the groups metadata of splits, I would like to follow up with another PR to this PR to actually make the default policy enabled by default when enable_metadata_routing=True and instead expose a new enabled_metadata_routing="empty_requests", enabled_metadta_routing="verbose_requests" or enabled_metadta_routing="explicit_requests" to implement the fully empty requests scheme.

This would require reworking the example further, so I prefer to decouple the two concerns to ease the review process.

adrinjalali · Mar 11, 2025

Doing your proposed change in BaseEstimator would affect all children, including third party estimators, in a non-equal-footing way. Especially when combining this with enable_metadata_routing=True, by default all metadata in sklearn that we care about is requested, but the metadata accepted by third party libs is not.

I personally prefer to have a more "explicit" way to enable default routing than saying enable_metadata_routing=True, but if we're doing this, to keep the consistency, I see two paths forward:

Request all metadata by default with enable_metadata_routing=True, which would include groups and sample_weight
Enabling this default routing for objects which actually have sample_weight somewhere. For instance, for score in ClassifierMixin and RegressorMixin. It should solve our issue w/o making "smart" decisions. We can then have a FitSampleWeightRequesterMixin to add to the ones who have fit methods with sample_weight, and add that explicitly to those estimators / estimator groups (like a base class for a group of estimators).

As for groups in split objects, I intentionally didn't include that, since there's a difference between groups and sample_weight. In split objects, they raise if groups is not requested. So there's no scenario where it can be not passed by the split object works; whereas sample_weight is always optional, and all objects who accept it, work w/o it. Therefore I think it makes sense to keep groups setting as is.

sklearn/_config.py

antoinebaker · Mar 18, 2025

I feel the current naming "default_routing" could be confusing because "default" is polysemic.
enable_metadata_routing="default_routing" means, as a request policy, to use the instance level requests as defaults. But it could be misleading to call it "default_routing", as it is not currently the default strategy, and isn't about the routing mechanics per se but more about the requests.

Maybe keeping enable_metadata_routing : bool, None as before, and adding a new config variable could help ?
Something like metadata_request_policy : str to choose among the various policies we would like to implement and test.
For instance metadata_request_policy = "empty", "explicit", "verbose" for the ones mentioned in #30946 (comment).

I am not sure how to name the policy implemented in this PR ("instance_defaults", "auto" ?) nor the old policy ("classwise_defaults" ?). But for backward compatibility we should maybe keep the old one as the default, until we are confident about the right policy and make it the new default in a future release ?

StefanieSenger · Mar 20, 2025

I feel the current naming "default_routing" could be confusing because "default" is polysemic.

I also think that's a problem.

Also the name of the new method/attribute __sklearn_default_request__ is so similar to the __metadata_request__* attributes, that it requires getting used to this part of the code to not constantly confuse them. It would be better to use "speaking names".

Just thinking aloud:

__sklearn_default_request__ could maybe be __sklearn_default_metadata_requests__ ?

__metadata_request__* could maybe be __sklearn_available_metadata_requests__*?

StefanieSenger · Mar 20, 2025

doc/whats_new/upcoming_changes/metadata-routing/30946.feature.rst

@@ -0,0 +1,7 @@
+- |Feature| Added support for default metadata routing through
+  ``set_config(enable_metadata_routing="default_routing")``. This enables
+  automatic routing of common metadata like ``sample_weight`` and ``groups`` by


Suggested change

automatic routing of common metadata like ``sample_weight`` and ``groups`` by

automatic routing of ``sample_weight`` and ``groups`` metadata (also ...) by

Can we be very explicit when communicating what exactly is included in the default routing? In the changelog and in the user guide, I would list all of them.

StefanieSenger · Mar 20, 2025

examples/miscellaneous/plot_metadata_routing.py

+    print_routing(clf)
+
+# %%
+# The routing can still be modified using set_*_request methods


Suggested change

# The routing can still be modified using set_*_request methods

# Users can still modify the routing using set_*_request methods

StefanieSenger · Mar 20, 2025

sklearn/utils/_metadata_requests.py

+             routing.
+
+        2. Before the user sets any specific routing, via `_get_default_requests`, it
+           provides the default metadata routing configuration for the instance. This


Suggested change

provides the default metadata routing configuration for the instance. This

provides the available metadata routing configuration for the instance. This

Maybe mention that it provides all the available routings.

StefanieSenger · Mar 20, 2025

sklearn/utils/_metadata_requests.py

+        For example, if a method's signature includes `sample_weight`, this method will:
+        - During class creation: Create a `set_{method}_request` method to configure
+          how `sample_weight` should be routed
+        - Right after initialization: Provide the default routing configuration for


Suggested change

- Right after initialization: Provide the default routing configuration for

- Right after initialization: Provide the available routing configuration for

StefanieSenger · Mar 20, 2025

sklearn/utils/_metadata_requests.py

+            for method, method_requests in defaults.items():
+                for pname, value in method_requests.items():
+                    getattr(requests, method).add_request(param=pname, alias=value)
+        return requests


What is the purpose of _get_default_requests() returning the class requests if _default_routing_enabled() is False?

Wouldn't it be a bug if an estimator doesn't implement their own default request and then as a result metadata is routed to all the methods where that is available in the routing?

Thinking about us then adding a new routing, and suddenly the default request would change.

Edit: Correcting myself here a bit: I have seen that this method gets called by _get_metadata_request() and of cause something needs to get passed there also if _default_routing_enabled() is False. But then I would think what gets passed is not a default request anymore. I am still concerned about the terminology.

StefanieSenger · Mar 20, 2025

examples/miscellaneous/plot_metadata_routing.py

+        # Instance-level default requests can override class-level defaults
+        # and add new method requests


Suggested change

# Instance-level default requests can override class-level defaults

# and add new method requests

# Sets default requests for the instance chosen from the available

# __metadata_request__* class attributes.

StefanieSenger · Mar 20, 2025

sklearn/utils/_metadata_requests.py

+           - Collecting metadata request info from `__metadata_request__*` class
+             attributes
+           - Analyzing method signatures for implicit metadata parameters
+             The collected information is used to create `set_{method}_request` methods
+             (e.g. set_fit_request) that allow runtime configuration of metadata
+             routing.


Suggested change

- Collecting metadata request info from `__metadata_request__*` class

attributes

- Analyzing method signatures for implicit metadata parameters

The collected information is used to create `set_{method}_request` methods

(e.g. set_fit_request) that allow runtime configuration of metadata

routing.

- Analyzing method signatures for implicit metadata parameters

The collected information is used to create `set_{method}_request` methods

(e.g. set_fit_request) that allow runtime configuration of metadata

routing.

- Collecting metadata request info from `__metadata_request__*` class

attributes

I would turn the order around to be the same as the methods does it (lowers mental load for third party developers who want to use these.

StefanieSenger · Mar 20, 2025

sklearn/utils/_metadata_requests.py

+             (e.g. set_fit_request) that allow runtime configuration of metadata
+             routing.
+
+        2. Before the user sets any specific routing, via `_get_default_requests`, it


Suggested change

2. Before the user sets any specific routing, via `_get_default_requests`, it

2. Before the user sets any specific routing, via `__sklearn_default_request__`, it

It is __sklearn_default_request__, isn't it?

StefanieSenger · Mar 20, 2025

sklearn/tests/test_metadata_routing.py

+        assert _default_routing_enabled() == default_routing
+
+
+def test_default_instance_routing_overrides_class_level():


There is already test_setting_default_requests() in this file and maybe these tests can be either merged or more clearer separated?

FEAT enable default routing strategy machinery

4e9c8a7

github-actions bot added module:model_selection module:utils labels Mar 5, 2025

adrinjalali added 5 commits March 5, 2025 17:52

cleanups

d4bd852

add example

2c38534

Merge remote-tracking branch 'upstream/main' into default_routing

53152e8

cleanup

d14200e

add changelog

fcbc431

adrinjalali marked this pull request as ready for review March 6, 2025 12:16

ogrisel self-requested a review March 7, 2025 19:14

virchan added the Metadata Routing all issues related to metadata routing, slep006, sample props label Mar 9, 2025

adrinjalali mentioned this pull request Mar 10, 2025

FIX LogisticRegressionCV.score and _BaseScorer metadata routing #30859

Open

ogrisel mentioned this pull request Mar 11, 2025

Default routing for sample weight adrinjalali/scikit-learn#5

Open

thomasjpfan reviewed Mar 17, 2025

View reviewed changes

sklearn/_config.py Show resolved Hide resolved

StefanieSenger reviewed Mar 20, 2025

View reviewed changes

adrinjalali mentioned this pull request May 20, 2025

FEAT allow configuring automatically requested metadata #31401

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT enable default routing strategy machinery #30946

FEAT enable default routing strategy machinery #30946

adrinjalali commented Mar 5, 2025

github-actions bot commented Mar 5, 2025 •

edited

Loading

adrinjalali commented Mar 6, 2025

ogrisel commented Mar 11, 2025

adrinjalali commented Mar 11, 2025

antoinebaker commented Mar 18, 2025 •

edited

Loading

StefanieSenger commented Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025

StefanieSenger Mar 20, 2025

StefanieSenger Mar 20, 2025 •

edited

Loading

	automatic routing of common metadata like ``sample_weight`` and ``groups`` by
	automatic routing of ``sample_weight`` and ``groups`` metadata (also ...) by

	# The routing can still be modified using set_*_request methods
	# Users can still modify the routing using set_*_request methods

	provides the default metadata routing configuration for the instance. This
	provides the available metadata routing configuration for the instance. This

	- Right after initialization: Provide the default routing configuration for
	- Right after initialization: Provide the available routing configuration for

		# Instance-level default requests can override class-level defaults
		# and add new method requests

	2. Before the user sets any specific routing, via `_get_default_requests`, it
	2. Before the user sets any specific routing, via `__sklearn_default_request__`, it

		assert _default_routing_enabled() == default_routing


		def test_default_instance_routing_overrides_class_level():

Search code, repositories, users, issues, pull requests...

FEAT enable default routing strategy machinery #30946

Are you sure you want to change the base?

FEAT enable default routing strategy machinery #30946

Conversation

adrinjalali commented Mar 5, 2025

github-actions bot commented Mar 5, 2025 • edited Loading

✔️ Linting Passed

adrinjalali commented Mar 6, 2025

ogrisel commented Mar 11, 2025

adrinjalali commented Mar 11, 2025

antoinebaker commented Mar 18, 2025 • edited Loading

StefanieSenger commented Mar 20, 2025 • edited Loading

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

StefanieSenger Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Mar 5, 2025 •

edited

Loading

antoinebaker commented Mar 18, 2025 •

edited

Loading

StefanieSenger commented Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading