FEAT enable default routing strategy machinery #30946

adrinjalali · 2025-03-05T10:53:25Z

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

github-actions · 2025-03-05T10:54:36Z

✔️ Linting Passed

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

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

adrinjalali · 2025-03-06T12:16:52Z

cc @antoinebaker @ogrisel @StefanieSenger

ogrisel · 2025-03-11T17:24:24Z

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 · 2025-03-11T18:37:48Z

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 · 2025-03-18T10:06:44Z

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 · 2025-03-20T12:27:05Z

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 · 2025-03-20T12:11:25Z

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 · 2025-03-20T13:07:59Z

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 · 2025-03-20T13:13:31Z

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

Default has too many meanings already, so I think it would be cool to find another term, such as available maybe?

StefanieSenger · 2025-03-20T13:14:35Z

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 · 2025-03-20T13:16:23Z

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 · 2025-03-20T13:50:49Z

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.

I don't understand my own suggestion anymore.

Better like this?

Suggested change

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

# and add new method requests

# Sets default requests for metadata routing at instance level.

StefanieSenger · 2025-03-20T14:20:16Z

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 · 2025-03-20T14:41:02Z

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?

Adding to this, I feel the term "user" here is ambiguous. The developers of any scikit-learn compatible estimators are meant here, if I get it right.

Suggested change

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

2. Before any specific routing gets configured at instance level via

`__sklearn_default_request__`, it

StefanieSenger · 2025-03-20T14:50:30Z

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?

StefanieSenger

I have gone over this again to be able to compare with #31401 and left some new comments. (Take them for what they are worth.)

Also after some time having passed, my main concern is that the term default is used to mean different states here.

StefanieSenger · 2025-05-21T10:48:24Z

examples/miscellaneous/plot_metadata_routing.py

+# There are two ways to set default metadata routing requests in scikit-learn:
+#
+# 1. Class-level defaults using `__metadata_request__method` class attributes,
+#    which sets default request values for all instances of a class, and can even
+#    remove a metadata from the metadata routing machinery if necessary.
+# 2. Instance-level defaults using `__sklearn_default_requests__` method,
+#    which set default request values at instance level, which means you can customize
+#    the default request values on a per-instance basis. Note that this only takes
+#    effect if ``set_config(enable_metadata_routing="default_routing")`` is called.


I have thought that I had understood this at one point, but I am not sure anymore:

So if a user (end user, not third party developer) enables "default_routing" in the configs, then a default routing strategy (which is defined at instance level by us or by a third party library) gets used.

But when is the class level default routing enabled then?

And why is it necessary to provide two different ways to set default requests? In my understanding, we can do everything and more on instance level alone.

StefanieSenger · 2025-05-21T10:58:14Z

examples/miscellaneous/plot_metadata_routing.py

+# 1. Class-level defaults using `__metadata_request__method` class attributes,
+#    which sets default request values for all instances of a class, and can even
+#    remove a metadata from the metadata routing machinery if necessary.


I don't quite get when the class level ~~routing~~ configuration would be enabled? On set_config(enable_metadata_routing=True)?

And what would it mean when it changes the "metadata routing machinery"? Would there be any routing with set_config(enable_metadata_routing=True) that is independent from what is defined in __metadata_request__method?

Edit: My confusion came from the fact that I thought that "remove a metadata from the metadata routing machinery" meant to set routing to it to False, and consequently they must have been set to True before.

Now I see, that class level configuration is only used in the MetadataRequest object, if set_config(enable_metadata_routing=True) and that "remove a metadata from the metadata routing machinery" actually means to disable users to set it at all by fully remove it, doesn't it?

--> If so, then I would think it's better either reformulate this or not to mention it here so prominently, since that would be a very special case and this half sentence is a bit confusing.

StefanieSenger · 2025-05-21T11:01:25Z

examples/miscellaneous/plot_metadata_routing.py

+
+class DefaultRoutingClassifier(ClassifierMixin, BaseEstimator):
+    # Class-level default request for fit method
+    __metadata_request__fit = {"sample_weight": True}


On naming __metadata_request__fit and the other attributes:

For better maintainability, I would pick names for these attributes that don't contain any method names.

These will show up for any contributor in scikit-learn and third party library upon inspection or debugging and the first thought would be that this is a callable method, when in fact it is not callable.

StefanieSenger · 2025-05-21T11:07:02Z

examples/miscellaneous/plot_metadata_routing.py

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


I don't understand my own suggestion anymore.

Better like this?

Suggested change

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

# and add new method requests

# Sets default requests for metadata routing at instance level.

StefanieSenger · 2025-05-21T11:31:19Z

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


Adding to this, I feel the term "user" here is ambiguous. The developers of any scikit-learn compatible estimators are meant here, if I get it right.

Suggested change

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

2. Before any specific routing gets configured at instance level via

`__sklearn_default_request__`, it

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

StefanieSenger reviewed May 21, 2025

View reviewed changes

	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

	# Instance-level default requests can override class-level defaults
	# and add new method requests
	# Sets default requests for metadata routing at instance level.

	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

	2. Before the user sets any specific routing, via `_get_default_requests`, it
	2. Before any specific routing gets configured at instance level via
	`__sklearn_default_request__`, it

		assert _default_routing_enabled() == default_routing


		def test_default_instance_routing_overrides_class_level():

Uh oh!

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

Uh oh!

github-actions bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

adrinjalali commented Mar 6, 2025

Uh oh!

ogrisel commented Mar 11, 2025

Uh oh!

adrinjalali commented Mar 11, 2025

Uh oh!

Uh oh!

antoinebaker commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StefanieSenger commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger May 21, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger left a comment

Choose a reason for hiding this comment

Uh oh!

StefanieSenger May 21, 2025

Choose a reason for hiding this comment

Uh oh!

StefanieSenger May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanieSenger May 21, 2025

Choose a reason for hiding this comment

Uh oh!

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 May 21, 2025 •

edited

Loading

StefanieSenger Mar 20, 2025 •

edited

Loading

StefanieSenger May 21, 2025 •

edited

Loading

StefanieSenger May 21, 2025 •

edited

Loading

StefanieSenger May 21, 2025 •

edited

Loading