-
-
Notifications
You must be signed in to change notification settings - Fork 25.5k
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
base: main
Are you sure you want to change the base?
ENH Expose verbose_feature_names_out in make_union #30406
Conversation
Signed-off-by: Abhijeetsingh Meena <[email protected]>
Signed-off-by: Abhijeetsingh Meena <[email protected]>
There was a problem hiding this 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]>
sklearn/tests/test_pipeline.py
Outdated
return mock | ||
|
||
|
||
def test_make_union_verbose_feature_names_out_enabled(): |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
Signed-off-by: Abhijeetsingh Meena <[email protected]>
Reference Issues/PRs
Fixes #30364
What does this implement/fix? Explain your changes.
This PR exposes the
verbose_feature_names_out
argument in themake_union
function. It allows users to control feature name uniqueness in theFeatureUnion
by enabling or disabling automatic name prefixing, which helps in managing feature name collisions more effectively.Any other comments?
No additional comments.