8000 ENH Add get_feature_names_out for ensemble module by MaxwellLZH · Pull Request #21459 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

ENH Add get_feature_names_out for ensemble module #21459

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

Closed

Conversation

MaxwellLZH
Copy link
Contributor

Reference Issues/PRs

Part of #21308

What does this implement/fix? Explain your changes.

Implementing get_feature_names_out for RandomForestEmbedding, VotingClassifier, VotingRegressor, StackingClassifier and StackingRegressor, with corresponding test cases.

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.

Here is a way to help you fix the broken CI but I think we should directly reuse _ClassNamePrefixFeaturesOutMixin for this module. See comments below for details:

return np.asarray(
[f"{class_name}{i}" for i in range(self.n_outputs_)],
dtype=object,
)
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, you could merge the current main into this branch to reuse the _ClassNamePrefixFeaturesOutMixin mixin-class introduced in https://github.com/scikit-learn/scikit-learn/pull/21334/files#diff-ac9c452d8c660d2516089c8448596912c22417c7c690f079715593e61ef6ee0dR884

This would require you to set the private _n_features_out attribute at fit time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ogrisel , I've changed all of the classes to use _ClassNamePrefixFeaturesOutMixin as you suggested

@ogrisel
Copy link
Member
ogrisel commented Nov 12, 2021

@MaxwellLZH it seems that you have added a bunch of unwanted changes to this branch when merging main and there are still conflicts.

@ogrisel
Copy link
Member
ogrisel commented Nov 18, 2021

This branch is still in an inconsistent state. Maybe start over in a new branch branched off of a recently updated main and open a new PR?

@MaxwellLZH
Copy link
Contributor Author

closed. Reopen a new PR.

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.

2 participants
0