Skip to content

ENH Adds get_feature_names_out for AdditiveChi2Sampler#22137

Merged
jeremiedbb merged 8 commits intoscikit-learn:mainfrom
thomasjpfan:feature_names_out_AdditiveChi2Sampler
Mar 8, 2022
Merged

ENH Adds get_feature_names_out for AdditiveChi2Sampler#22137
jeremiedbb merged 8 commits intoscikit-learn:mainfrom
thomasjpfan:feature_names_out_AdditiveChi2Sampler

Conversation

@thomasjpfan
Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan commented Jan 6, 2022

Reference Issues/PRs

Towards #21308

What does this implement/fix? Explain your changes.

This PR is adds get_feature_names_out for AdditiveChi2Sampler. This is it's own PR because the generated names are specific to this transformer.

Any other comments?

Docstring for transform is incorrect. I opened #22138 as a quick docstring fix.

The test_transformers_get_feature_names_out common tests will run on AdditiveChi2Sampler because it defines get_feature_names_out in this PR.

def _include_in_get_feature_names_out_check(transformer):
if hasattr(transformer, "get_feature_names_out"):
return True

Copy link
Copy Markdown
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

LGTM. The output feature names are actually very informative for this transformer. Just a comment on the changelog:

Co-authored-by: Olivier Grisel <[email protected]>
@ogrisel ogrisel added the Quick Review For PRs that are quick to review label Mar 7, 2022
Copy link
Copy Markdown
Member

@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

I directly resolved the conflicts from merging #22694 first. LGTM. Thanks @thomasjpfan !

@jeremiedbb jeremiedbb merged commit 67a3fee into scikit-learn:main Mar 8, 2022
thomasjpfan added a commit to thomasjpfan/scikit-learn that referenced this pull request Mar 8, 2022
…22137)

Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Jérémie du Boisberranger <[email protected]>
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Apr 6, 2022
…22137)

Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Jérémie du Boisberranger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Quick Review For PRs that are quick to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants