Skip to content

FIX FunctionTransformer overwrite column names if not consistent#28241

Merged
thomasjpfan merged 16 commits into
scikit-learn:mainfrom
glemaitre:is/28232
Feb 1, 2024
Merged

FIX FunctionTransformer overwrite column names if not consistent#28241
thomasjpfan merged 16 commits into
scikit-learn:mainfrom
glemaitre:is/28232

Conversation

@glemaitre

Copy link
Copy Markdown
Member

closes #28232

This make the FunctionTransformer more lenient by overwriting columns if the output is not consistent with the behaviour of get_feature_names_out. We were previously raising an error instead but we used this inconsistency within the ColumnTransformer.

It remains one question: do we want to trigger a copy when setting the columns? I think it is weird that the input X get modified once passed in the FunctionTransformer.

@github-actions

github-actions Bot commented Jan 24, 2024

Copy link
Copy Markdown

✔️ Linting Passed

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

Generated for commit: 6cc0ea8. Link to the linter CI: here

@glemaitre glemaitre added this to the 1.4.1 milestone Jan 24, 2024
Comment thread sklearn/preprocessing/_function_transformer.py Outdated

@adrinjalali adrinjalali left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ended up a large diff 😁

Comment thread sklearn/compose/tests/test_column_transformer.py
Comment thread sklearn/preprocessing/_function_transformer.py Outdated
Comment thread sklearn/preprocessing/_function_transformer.py Outdated
Comment thread sklearn/preprocessing/_function_transformer.py Outdated
Comment thread sklearn/utils/_set_output.py Outdated
@glemaitre

glemaitre commented Jan 31, 2024

Copy link
Copy Markdown
Member Author

The failure will be solved by merging #28262

@adrinjalali adrinjalali left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise LGTM.

Comment thread sklearn/preprocessing/_function_transformer.py Outdated
Comment thread sklearn/preprocessing/_function_transformer.py
Comment thread sklearn/preprocessing/_function_transformer.py Outdated
Comment thread sklearn/utils/tests/test_set_output.py Outdated
Comment thread sklearn/utils/tests/test_set_output.py Outdated
Comment thread sklearn/utils/tests/test_set_output.py Outdated
Comment thread sklearn/utils/tests/test_set_output.py Outdated
Comment thread sklearn/preprocessing/_function_transformer.py Outdated
@celestinoxp

Copy link
Copy Markdown

As I mentioned before, my pull-request in pycaret to have scikit-learn 1.4 support failed, but now I have just modified the scikit-learn code on my laptop with the modifications in this PR, so far, I have run the scikit-learn test again pycaret that failed because of get_feature_names_out and now the test passed!
So I'm commenting to give my testimony with the aim of collaborating.

For more context, I refer to my Pycaret pr with failing tests because of get_feature_names_out, it's here: pycaret/pycaret#3857

@thomasjpfan thomasjpfan added the To backport PR merged in master that need a backport to a release branch defined based on the milestone. label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:compose module:preprocessing To backport PR merged in master that need a backport to a release branch defined based on the milestone.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression in ColumnTransformer due to internal FunctionTransformer

4 participants