ENH Add metadata routing for FeatureUnion

[StefanieSenger]

Reference Issues/PRs

Towards #22893

What does this implement/fix? Explain your changes.

This PR adds metadata routing to FeatureUnion and the corresponding tests.

I was confused about why we cannot do set_fit_transform_request. I know we can work around it, but from a user's perspective it's not self-explanatory.

[github-actions]

✔️ Linting Passed

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

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

[adrinjalali]

set_fit_transform_request isn't well-defined since it's a composite method, therefore it always has to be a combination of routing for fit and transform.

[adrinjalali]

Also the test about making sure get_metadata_routing works on an unfitted estimator here.

[adrinjalali]

we don't do explicit fit_transform here, since it's a compound method.

[StefanieSenger]

Now I see:

From FeatureUnion's fit_transform we need to prepare for routing into three directions: to fit, to transform and to fit_transform. This is because of the way _fit_transform_one is designed:

        if hasattr(transformer, "fit_transform"):
            res = transformer.fit_transform(X, y, **params.get("fit_transform", {}))
        else:
            res = transformer.fit(X, y, **params.get("fit", {})).transform(
                X, **params.get("transform", {})
            )

It is designed this way, because a transformer can use either fit and transform or fit_transform and in fact, metadata routing is only possible, if it overwrites TransformerMixin's fit_transform.

But yes, we don't route from the FeaturUnion's fit to the estimator's fit_transform.

I've changed the routing to my new understandings.

[StefanieSenger]

We actually don't have a test for transformer.fit().transform(). I made one using a new class ConsumingNoFitTransformTransformer, that is special, because it doesn't inherit from TransformerMixin. it has a long name, but it's a speaking name. :)

We need this new class to make transformer.fit().transform() possible.

ConsumingClassifier's fit and transform would not be used, because it already has a fit_transform, that would be used instead.

[adrinjalali]

Cool!

[adrinjalali]

Suggested change

	routed_fit_transform_params[name].fit_transform = fit_params
	routed_fit_transform_params[name].fit_transform = fit_params
	routed_fit_transform_params[name].fit = fit

[StefanieSenger]

I believe it needs to be like that:

                if hasattr(obj, "fit_transform"):
                    routed_params[name] = Bunch(fit_transform={})
                    routed_params[name].fit_transform = params
                else:
                    routed_params[name] = Bunch(fit={})
                    routed_params[name] = Bunch(transform={})
                    routed_params[name].fit = params
                    routed_params[name].transform = params

It accounts for the metadata to be passed to the correct methods of the sub-transformer. I've also added a test for that (test_feature_union_fit_params_without_fit_transform).

[StefanieSenger]

I've addressed all your review comments, @adrinjalali.
Would you have a fresh look if the PR is alright now?

[adrinjalali]

Otherwise LGTM.

[StefanieSenger]

Okay, I've taken care of that, @adrinjalali.

[glemaitre]

It looks good. Just some questions reagarding transform and thus the renaming from fit_params to params.

[adrinjalali]

Otherwise LGTM.

[glemaitre]

@StefanieSenger I think that we are missing forwarding the metadata in the transform method of FeatureUnion. Quickly speaking with @adrinjalali, it seems this is something that we overlooked here.

[StefanieSenger]

Oh yes, @glemaitre and @adrinjalali, sorry for the oversight.
I've added that now.

[StefanieSenger]

I've also simplified the Metadata Router of FeatureUnion (by aligning if and else conditions), because they are almost identical and I think no harm would happen, by providing more possibilities than an object would use.

[glemaitre]

LGTM. Thanks @StefanieSenger