✔️ Linting Passed

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

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

I wonder if I can have feedback before I add/fix more tests.
@glemaitre

I see that we have to handle one specific case:

We have an internal PassThrough transformer that forward the input feature as-is and this it means that we should mention that the output feature are the same as n_features_in_.

I find the block a bit big, it takes as much space as the estimator itself. I was also thinking that having the input features would be nice but then it really starts to take a lot of space around the estimator. So I wondered if the features could be intermediate blocks in the diagram, representing both the output features from the previous estimator and the input features for the next estimator. Something like this

This way the diagram alternates estimator blocks and data blocks
Then the text would be different obviously, like "16 features", or even the full shape ?

In addition, in this PR or in a following one, the data block could be unfold to show the feature names if available.

One feedback of @ogrisel IRL is to directly show the feature names using the same pattern than "Parameters".

I personally agree with @jeremiedbb feedback: I would like something smaller. Also write now, we have to mention "output features" instead of simply "features" because of the ambiguity input/output when attached to the estimator. So the proposal to make the "feature" being blocks leaving on their own is nice I think because there is not ambiguity anymore.

I'll work on this, thanks for the feedback. Just:

One feedback of @ogrisel IRL is to directly show the feature names using the same pattern than "Parameters".

Should I add the feature names on this PR? I remember @glemaitre saying that they should be added on a separate PR.

Should I add the feature names on this PR?

I want to dissociate it at first but since we are going to create a new block, it might be better to have directly the feature names as well.

Hi @DeaMariaLeon, here is first pass of review (I haven't looked at the tests yet).

I don't see any changes made to plot_column_transformer_mixed_types. Could you try set_output(transform="pandas") as suggested here #31937 (comment) and as you have already done for plot_cyclical_feature_engineering ?

I thought that his comment was just an explanation on the question you had. I didn't understand I should actually make the change. I'll do it.

EDIT: Do you know how to do that? I either get an error, or keep getting just the "x0, x4, x5" etc. @antoinebaker

I did reply to #31937 (comment), but it's only visible on github's "Files changed".

In this comment: #31937 (comment)
I wrote an "EDIT" that may be difficult to see. So I'll add that here just in case:

Hi @DeaMariaLeon, here is first pass of review (I haven't looked at the tests yet).

I don't see any changes made to plot_column_transformer_mixed_types. Could you try set_output(transform="pandas") as suggested here #31937 (comment) and as you have already done for plot_cyclical_feature_engineering ?

Me:

I thought that his comment was just an explanation on the question you had. I didn't understand I should actually make the change. I'll do it.

Me again:
Do you know how to do that? I either get an error, or keep getting just the "x0, x4, x5" etc. @antoinebaker

Do you know how to do that? I either get an error, or keep getting just the "x0, x4, x5" etc. @antoinebaker

Well if the set_output(transform="pandas") does not work, don't bother :) Could you instead create a new issue with a screeenshot for this example ? (after this PR is merged)

Well if the set_output(transform="pandas") does not work, don't bother :) Could you instead create a new issue with a screeenshot for this example ? (after this PR is merged)

Looking at this again, I fail to see the issue. On that particular example (plot_column_transformer_mixed_types), the input of SelectPercentile are not the original features. Its inputs are already transformed by OneHotEncoder, so how can it give the names of the original columns? I think that what it shows is correct, but I may be missing something.

Its inputs are already transformed by OneHotEncoder, so how can it give the names of the original columns?

Not the original names but the names output by the OneHotEncoder:

sex, pclass -> OneHotEncoder -> sex_female, sex_male, pclass_1, pclass_2, ... -> SelectPercentile -> sex_female, sex_male, pclass_1, pclass_3

which makes the preprocessing much easier to follow that "anonymous" column names such as x0, x1, ...

You are right, it would be easier.

But it works:

I mean: it works with a small change to the example.

EDIT (note for myself): There was a known issue with OneHotEncoder, and needed to set sparse=False to it on the example. That way one can use set_output(transform="pandas") and don't brake (the example).

Example can be seen from the built docs:
https://output.circle-artifacts.com/output/job/e9b6714c-0349-4347-8f40-4becd58d9e0d/artifacts/0/doc/auto_examples/compose/plot_column_transformer_mixed_types.html

Up to here, I think I have added all the feedback from @antoinebaker except #31937 (comment) because of the circular import.

EDIT: Imported ColumnTransformer as suggested.

Thanks @antoinebaker!