Skip to content
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

ENH Expose verbose_feature_names_out in make_union #30406

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Ethan0456
Copy link

Reference Issues/PRs

Fixes #30364

What does this implement/fix? Explain your changes.

This PR exposes the verbose_feature_names_out argument in the make_union function. It allows users to control feature name uniqueness in the FeatureUnion by enabling or disabling automatic name prefixing, which helps in managing feature name collisions more effectively.

Any other comments?

No additional comments.

Copy link

github-actions bot commented Dec 4, 2024

✔️ Linting Passed

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

Generated for commit: 7bd03c6. Link to the linter CI: here

Signed-off-by: Abhijeetsingh Meena <[email protected]>
Copy link
Member

@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @Ethan0456 !

  • Can you add a test to sklearn/tests/test_pipeline.py?
  • Also add a whats new in doc/whats_new/upcoming_changes/sklearn.pipeline/30406.enhancement.rst. You can follow the pattern that directory to see how to create new entry.

…t in make_union

- Added a test case to validate the `verbose_feature_names_out` argument in `make_union`, including handling of duplicate feature names.
- Added an enhancement entry in `doc/whats_new/upcoming_changes/sklearn.pipeline/30406.enhancement.rst`.

References scikit-learn#30364

Signed-off-by: Abhijeetsingh Meena <[email protected]>
Signed-off-by: Abhijeetsingh Meena <[email protected]>
return mock


def test_make_union_verbose_feature_names_out_enabled():
Copy link
Member

Choose a reason for hiding this comment

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

Since, we already have similar test for FeatureUnion. I think it's enough to test that make_union sets verbose_feature_names_out correctly. Something like this:

def test_make_union_passes_verbose_feature_names_out():
    union = make_union(..., verbose_feature_names_out=False)
    assert not union.verbose_feature_names_out

Copy link
Author

Choose a reason for hiding this comment

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

Hi @thomasjpfan,

I've simplified the test case in my recent commit. Could you please review it when you get a chance?

Thanks!

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.

Expose verbose_feature_names_out in make_union
2 participants