Skip to content

Conversation

thomasjpfan
Copy link
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
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 <olivier.grisel@gmail.com>
@ogrisel ogrisel added the Quick Review For PRs that are quick to review label Mar 7, 2022
Copy link
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 <olivier.grisel@gmail.com>
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Apr 6, 2022
…22137)

Co-authored-by: Olivier Grisel <olivier.grisel@gmail.com>
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
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