Here's an example with a pipeline. @jnothman would you please check if it's [almost] what you had in mind?

@NicolasHug you were interested in this one as well IIRC.

from sklearn.model_selection import GroupKFold, cross_validate
from sklearn.feature_selection import SelectKBest
from sklearn.pipeline import make_pipeline
from sklearn.linear_model import LogisticRegressionCV
from sklearn.metrics import accuracy_score
from sklearn.metrics import make_scorer
import numpy as np
from sklearn.datasets import make_classification
X, y = make_classification()
sw = np.random.rand(len(X))
sw2 = [5, 6]
brand = ['my brand']

from sklearn.base import BaseEstimator, ClassifierMixin, TransformerMixin
from sklearn.utils.validation import _validate_required_props
from sklearn.svm import SVC

class MyEst(ClassifierMixin, BaseEstimator):
    def __init__(self):
        self._props_request = {'fit': ['sample_weight', 'brand']}

    def fit(self, X, y, **fit_params):
        _validate_required_props(self, fit_params, 'fit')
        self._estimator = SVC().fit(X, y, fit_params['sample_weight'])
        print("{} recieved params {} of lengths {}".format(
            self.__class__.__name__,
            list(fit_params.keys()), [len(v) for v in fit_params.values()]))

class MyTrs(TransformerMixin, BaseEstimator):
    def __init__(self):
        self._props_request = {'fit': ['sample_weight']}

    def fit(self, X, y=None, **fit_params):
        _validate_required_props(self, fit_params, 'fit')
        self._estimator = SelectKBest().fit(X, y)
        print("{} recieved params {} of lengths {}".format(
            self.__class__.__name__,
            list(fit_params.keys()), [len(v) for v in fit_params.values()]))
        return self

    def transform(self, X, y=None):
        return self._estimator.transform(X)

clf = make_pipeline(MyTrs(), MyEst())
clf.fit(X, y, sample_weight=sw, brand=brand)

MyTrs recieved params ['sample_weight'] of lengths [100]
MyEst recieved params ['sample_weight', 'brand'] of lengths [100, 1]

Pipeline(memory=None, steps=[('mytrs', MyTrs()), ('myest', MyEst())],
         verbose=False)

trs = MyTrs().set_props_request(
    None).set_props_request(
    {'fit': {'new_param': 'my_sw'}})
clf = make_pipeline(trs, MyEst())
clf.fit(X, y, sample_weight=sw, brand=brand, my_sw=sw2)

MyTrs recieved params ['new_param'] of lengths [2]
MyEst recieved params ['sample_weight', 'brand'] of lengths [100, 1]

Pipeline(memory=None, steps=[('mytrs', MyTrs()), ('myest', MyEst())],
         verbose=False)

clf.fit(X, y, brand=brand)

---------------------------------------------------------------------------

ValueError                                Traceback (most recent call last)

<ipython-input-6-b2a96948c309> in <module>
----> 1 clf.fit(X, y, brand=brand)


~/Projects/sklearn/scikit-learn/sklearn/pipeline.py in fit(self, X, y, **fit_params)
    364             This estimator
    365         """
--> 366         Xt, fit_params = self._fit(X, y, **fit_params)
    367         with _print_elapsed_time('Pipeline',
    368                                  self._log_message(len(self.steps) - 1)):


~/Projects/sklearn/scikit-learn/sklearn/pipeline.py in _fit(self, X, y, **fit_params)
    288         fit_transform_one_cached = memory.cache(_fit_transform_one)
    289 
--> 290         _validate_required_props(self, fit_params, 'fit')
    291 
    292         for (step_idx,


~/Projects/sklearn/scikit-learn/sklearn/utils/validation.py in _validate_required_props(obj, given_props, method)
   1337     given_props = set(given_props.keys())
   1338     if required_props != given_props:
-> 1339         raise ValueError("Requested properties are: {}, but {} "
   1340                          "provided".format(list(required_props),
   1341                                            list(given_props)))


ValueError: Requested properties are: ['brand', 'my_sw', 'sample_weight'], but ['brand'] provided

That example looks nice! I think I had not envisaged that all requested props were required, i.e. an estimator could request sample_weight by default but only receive it if available. What are your thoughts on that?

What I'm not sure about is whether we need to specify that the request is for fit. I suppose that makes things more flexible.

I have looked at your modification to clone. Okay. But I've not yet looked at much else

I think I had not envisaged that all requested props were required, i.e. an estimator could request sample_weight by default but only receive it if available. What are your thoughts on that?

I also first thought of having it as "please give me that prop if you have it" rather than "I really need it". The reason I have it as a mandatory requirement, is that it kinda removes the chances of a meta estimator not passing a required prop by mistake. My gut feeling was that it removes some of those bugs.

What I'm not sure about is whether we need to specify that the request is for fit. I suppose that makes things more flexible.

I think your other proposals had a more flexible system for routing the props around. Especially when it comes to fit/score/split distinction. Also, I thought this makes it easier to have some props going to transform/predict as well.

The reason I have it as a mandatory requirement, is that it kinda removes the chances of a meta estimator not passing a required prop by mistake. My gut feeling was that it removes some of those bugs.

But this would be a mistake in meta-estimator implementation which should not be the user's problem?

This looks good!

Edit: previous comment was irrelevant after I finally read all of #4497 and #SLEP006. My initial conception (and hacky implementation in personal project) looked very similar to the single argument, no routing approach.

Now for some comments (sorry if these are still uninformed):

• single argument vs. kwargs: I feel strongly that the single argument approach is best.
• routing vs. blind passing of all: I see pros and cons to each, but I think it should be possible to implement in a non-exclusive manner. Ex, if the key starts with route_ it gets routed (however that ends up working) and if not it always gets passed. That would also allow for the simpler behavior to be implemented immediately and the routing to be done separately once that basic infrastructure is in place.
• unexpected behavior changes: very good points were raised in [API] Consistent API for attaching properties to samples #4497 regarding changes in behavior without changes in data. I think that is definitely a concern, but I cannot immediately think of a way around that.

I guess my main question/point is: can this be done in two phases? Phase 1 would be implementing the basic infrastructure (i.e. changing a lot of signatures thought the API) so that sample_props gets passed correctly. Changes would also have to be made to fold in sample_weight and other existing sample properties. Phase 2 would be routing, requesting/requiring, etc.

That said, how can I be of use to get this done?

I think the consensus at the last meeting was to rename the prop request to a metadata request.

How is this going? Is there something I can help with here?

I don't like adding these parameters to the constructor. I'm still hoping we'd use this as a starting point for not having all parameters as constructor parameters.

Other than that, the API you propose also doesn't have the flexibility for the routing we have right now. You can't route different sample weights to different estimators or different metadata to different verbs (fit, transform, etc), which the user might want to control and not leave it to the developer of the estimator.

I'm happy to introduce helper functions such as set_sampleweight_request etc to make it easier for people to set the routing for the usual cases though.

Other than that, the API you propose also doesn't have the flexibility for the routing we have right now. You can't route different sample weights to different estimators or different metadata to different verbs (fit, transform, etc), which the user might want to control and not leave it to the developer of the estimator.

Yes, the assumption is that the verbs relevant to a parameter is determined by the meaning of the parameter to that consumer. So that parameter knows how to consume sample_weight in fit, it will consume it in fit. You're right that this might be a problem for sample_weight in score.

A possibility: A method called request_sample_weight could more explicitly define which verbs sample_weight can be requested for ...?

How about this kind of thing for consumers?:

class MetadataConsumer:
	def _request_key_for_method(self, method, key, value):
		method_dict = self._metadata_request[method]
		if value is True:
			method_dict[key] = key
		elif value is False:
			del method_dict[key]
		elif isinstance(value, str):
			method_dict[key] = value
		else:
			raise TypeError

class SampleWeightConsumer(MetadataConsumer):
    def request_sample_weight(*, fit=True, score=False):
        """Define how to receive sample_weight from a parent meta-estimator

        When called with default arguments, fitting will be weighted,
        and the meta-estimator should be passed a fit parameter by the name
        'sample_weight'.

        Parameters
        ----------
        fit : string or bool, default=True
            The fit parameter name that a meta-estimator should pass to this
            estimator as sample_weight. If true, the name will be 'sample_weight'.
            If False, no fit parameter will be passed.

        score : string or bool, default=True
            The parameter name that a meta-estimator should pass to this
            estimator as sample_weight when calling its `score` method.
            If true, the name will be 'sample_weight'.
            If False, no fit parameter will be passed.

        Returns
        -------
        self

        Examples
        --------
        >>> from sklearn.linear_model import LogisticRegression
        >>> from sklearn.model_selection import GridSearchCV
        >>> from sklearn.datasets import load_iris
        >>> X, y = load_iris(return_X_y=True)
        >>> sample_weight = np.random.rand(len(X))
        >>> clf = LogisticRegression()
        >>> gs = GridSearchCV(clf, {"C": [.1, 1, 10]})
        # Unweighted fitting and scoring
        >>> gs.fit(X, y)
        # Weighted fitting, unweighted scoring
        >>> clf.request_sample_weight()
        >>> gs.fit(X, y, sample_weight=sample_weight)
        # Weighted fitting and scoring
        >>> clf.request_sample_weight(fit=True, score=True)
        >>> gs.fit(X, y, sample_weight=sample_weight)
        # Weighted scoring only
        >>> clf.request_sample_weight(fit=False, score=True)
        >>> gs.fit(X, y, sample_weight=sample_weight)
        # Distinct weights for fit and score
        >>> score_sample_weight = np.random.rand(len(X))
        >>> clf.request_sample_weight(fit='fit_sample_weight', score='score_sample_weight')
        >>> gs.fit(X, y, fit_sample_weight=sample_weight,
        ...        score_sample_weight=score_sample_weight)
        """
        self._request_key_for_method('fit', 'sample_weight', fit)
        self._request_key_for_method('score', 'sample_weight', score)
        return self

I think we need that deep parameter for clone. set_metadata_request needs to only pertain to the needs of that estimator as consumer, not as router, and this is what should be copied in clone. However, for the purpose of actually doing the routing, there needs to be a way to get the "deep" metadata request.

I copy the variable itself now, this should solve the deep/shallow issue.

I think we should keep set_metadata_request, if only because of its use in clone, but we might emphasise that most users should not use it as request_* methods should be a simpler, more explicit way of accessing the same functionality.

Need the method in metrics, but seems like we could do w/o for the other estimators. Happy to put it back. Made it private for now, since it's only used in make_scorer.

But I'm struggling to work out how to neatly make request_sample_weight appear in only the consumers of sample_weight.

We just need to use the mixin only where it's appropriate. I'm not sure if I get where you see the challenge here.

I'd now tweak the example in the previous comment such that neither fit nor score defaults to True or False. Both should default to None, and no change should occur.

Yes, agreed. There's another challenge which is to have a set of defaults in different mixins, and have get_metadata_request look for them in a general way. w/o having the user ever call a set_*. Update: Done in 6d27c79

I'm not totally happy with how it's done in cross_validate now, it'd be nice if you could let me know what you think @jnothman

The tests now include the 4 usecases presented in the SLEP and they run, but need to add tests to see if they're run correctly.

SLEP000 has a "provisionally accepted":

To allow gathering of additional design and interface feedback before
committing to long term stability for a feature or API, a SLEP may also be
marked as Provisional. This is short for "Provisionally Accepted", and
indicates that the proposal has been accepted for inclusion in the reference
implementation, but additional user feedback is needed before the full design
can be considered Final. Unlike regular accepted SLEPs, provisionally
accepted SLEPs may still be Rejected or Withdrawn even after the
related changes have been included in a scikit-learn release.

I feel the safe choice would be to reject and stay in "Draft":

If the vote does not achieve a required majority, then the SLEP remains in
Draft state, discussion continues as normal, and it can be proposed for
acceptance again later once the objections are resolved.

This way some of the concerns here can be resolved in the SLEP.

https://github.com/scikit-learn/enhancement_proposals/pull/30/files

Hey @adrinjalali, I'd like to have this closer to the SLEP when we next bring it to vote, and I wonder whether:

• you could put together a to do list so it's clear to us all
• we could consider management strategies to expedite it, e.g. allocating a few days to sprint on it, with either pair programming or fast review turnaround

@jnothman I'm happy to do both. I'm working on a cleaner solution for the same API. I'd be happy to have a sprint to work on this together as well.

Just to leave our scribbles from yesterday:

# A messy case

# One option in grid search is weighted, the other is unweighted

# Messy because when do you work out all the requests, and that all metadata
# are consumed?

GridSearchCV(
    Pipeline([
        ('clf', None)
    ]),
    param_grid={"clf": [SVM().fit_requests(sample_weight=True),
                        SVM().fit_requests(sample_weight=False)],
                "clf__C": [1,2,3],
                }
).fit(X, y, sample_weight=sample_weight)



###############

# Request type behaviour

class UnchangedSingleton:
    __dict__ = None

    def __repr__(self):
        return "<UNCHANGED>"


from enum import Enum

class RequestType(Enum):
    UNREQUESTED = False
    REQUESTED = True
    ERROR_IF_PASSED = None

UNCHANGED = UnchangedSingleton()


def fit_requests(self,
                 sample_weight : Union[RequestType, str, Literal[UNCHANGED]] = UNCHANGED):


fit_requests(sample_weight=metadata.REQUESTED)
fit_requests(sample_weight=metadata.UNREQUESTED)
fit_requests(sample_weight=metadata.ERROR_IF_PASSED)
fit_requests(sample_weight='my_weight')

###############

# Magic creation of `fit_requests` etc.

class RequestMethod:
    def __init__(self, name, keys):
        self.name = name
        self.keys = keys

    def __get__(self, instance, owner):
        # what we are avoiding
        def func(**kw):
            if set(kw) - keys:
                raise TypeError("Unexpected kws")

            instance._metadata_request = ....

            return instance

        func.__name__ = f"{self.name}_requests"
        func.__signature__ = inspect.Signature([
            inspect.Parameter(k,
                              inspect.Parameter.KEYWORD_ONLY,
                              default=metadata.UNCHANGED,
                              annotation=Optional[Union[RequestType, str]])
            for k in keys
        ], return_annotation=type(instance))
        func.__doc__ = "Smething useful"
        return func


class BaseEstimator:
    def __init_subclass__(cls, **kwargs):
        metadata = defaultdict(set)
        for k, v in vars(cls).items():
            if k.startswith("_metadata_re,
                            annotation=quest___"):
                for request_method, request_keys in v:
                    metadata[request_method].update(request_keys)

        # now we have something like
        # metadata = {"fit": {"sample_weight", "groups"}}

        for request_method, request_keys in metadata:
            # set a fit_requests method
            setattr(f"{request_method}_requests",
                    RequestMethod(request_method, sorted(request_keys)))


        """
        REQUEST_METHOD_MAPPING = [
            ('fit', "fit"),
            ("fit_transform", "fit"),
            ("fit_transform", "transform"),
            ("transform", "transform"),
        ]
        metadata = defaultdict(set)
        for method, request_method_name in REQUEST_METHOD_MAPPING:
            if method in attrs:
                # for example, if the class being instantiated has `fit`
                sig = inspect.signature(attrs[method])
                metadata[request_method_name].add()
        """
        super().__init_subclass__(**attrs)

The three request states makes a sense. I do not see a clean way to avoid using the sentinel value (reminds me of PEP661).

I do not see a clean way to avoid using the sentinel value

Well, we can avoid the use of None for ERROR_IF_CHANGED, and use a sentinel for that instead. That would allow us to use Optional

Well, we can avoid the use of None for ERROR_IF_CHANGED, and use a sentinel for that instead. That would allow us to use Optional

What do we use for ERROR_IF_PASSED instead of None?

I'm assuming we still want fit_requests(sample_weight=True) to mean fit_requests(sample_weight=metadata.REQUESTED), so users do not need to import metadata to set their request. Strings are already used for key aliasing.

Edit: I meant ERROR_IF_PASSED

I think "error if passed" is more explicit than None. Maybe error-if-passed since dashes have limited use in python for naming things.

It will only be seen by users who call get_metadata_request.

From the original SLEP, I was under the impression that get_metadata_request is empty when no metadata is requested.

# No metadata requested
log_reg = LogisticRegression()
log_reg.get_metadata_request()
# {'fit': {}, 'predict': {}, 'transform': {}, 'score': {}, 'split': {}, 'inverse_transform': {}}

Do we want fit to contain "error-if-passed" for sample weights?

log_reg.get_metadata_request()
# {'fit': {'sample_weight': 'error-if-passed'}, ...}

I've put an alternate implementation here: #20350

It is closer to what we have in the slep, and includes ERROR_IF_PASSED