Skip to content

FIX ColumnTransformer metadata routing work within another meta-estimator #28188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Feb 1, 2024

Conversation

adrinjalali
Copy link
Member

Fixes #28186

The core of the issue is that ColumnTransformer().get_metadata_request() raises if called before fit. This fixes the issue.

Note that this means all metadata requested by all given transformers are always requested, no matter whether those transformers are going to be used or not, due to column selections.

Copy link

github-actions bot commented Jan 19, 2024

✔️ Linting Passed

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

Generated for commit: 3272c34. Link to the linter CI: here

@adrinjalali adrinjalali added this to the 1.4.1 milestone Jan 19, 2024
@adrinjalali
Copy link
Member Author

cc @OmarManzoor @glemaitre

Copy link
Contributor

@OmarManzoor OmarManzoor left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @adrinjalali

DataFrame Support
-----------------

- |Enhancement| Pandas and Polars dataframe are validated directly without ducktyping
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this part of this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, was under the wrong section, but I had forgotten to remove the duplicate. Fixed now.

@glemaitre
Copy link
Member

I fixed the issue with the changelog and enabling auto-merge. Thanks @adrinjalali

@glemaitre glemaitre enabled auto-merge (squash) February 1, 2024 17:17
@glemaitre glemaitre merged commit 61068a7 into scikit-learn:main Feb 1, 2024
@adrinjalali adrinjalali deleted the slep6/fix-ct branch February 1, 2024 18:20
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 10, 2024
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Feb 13, 2024
glemaitre added a commit that referenced this pull request Feb 13, 2024
…ator (#28188)

Co-authored-by: Loïc Estève <[email protected]>
Co-authored-by: Guillaume Lemaitre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metadata Routing breaks ColumnTransformer
4 participants