-
-
Notifications
You must be signed in to change notification settings - Fork 26.2k
ENH Adds get_feature_names_out to neighbors module #22212
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 Adds get_feature_names_out to neighbors module #22212
Conversation
Initial implementation of `get_feature_names_out`
Since both transformers map from a collection of feature vectors to a distance graph, the "output features" would just be the distance to each point of the fitted data right? This would be very verbose and potentially unwieldy for what I imagine the intended use of So the two obvious (not necessarily great) solutions to this I see are:
Part of me thinks the more "romantic" solution would be to have I'm leaning towards the first option (output features = columns of distance graph). |
For |
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.
Thanks for the PR!
- Moved `_ClassNamePrefixFeaturesOutMixin` to specific transformers - Added `_ClassNamePrefixFeaturesOutMixin` to NCA - Added `self._n_features_out` to NCA
I guess my question is more-so: since the outputs of |
Yup, the number of feature names out must match the amount of output features from |
get_feature_names_out
to neighbors moduleget_feature_names_out
to neighbors module
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.
https://github.com/scikit-learn/scikit-learn/pull/22212/files#r785147042 still needs to be addressed. Once done, I think this PR LGTM.
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.
Thanks for the update @Micky774 !
We need tests to check the actual names of get_feature_names_out
. See:
scikit-learn/sklearn/cross_decomposition/tests/test_pls.py
Lines 613 to 614 in 1d69784
def test_pls_feature_names_out(Klass): | |
"""Check `get_feature_names_out` cross_decomposition module.""" |
Added in the unit tests! Thank you for all your active feedback, and for your patience in this. Let me know if there's anything else I should add here! |
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.
I merged the main
branch to resolve the conflict in the changelog. I also made this an enhancement instead of an API change because those features names method did not exist under a different name in the last released scikit-learn version.
One more code style detail, but otherwise, LGTM.
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.
I wanted to approve this PR in previous review, instead of just commenting.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
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.
LGTM
get_feature_names_out
to neighbors module
Reference Issues/PRs
Addresses #21308
What does this implement/fix? Explain your changes.
Implements the
get_feature_names_out
method to the neighbors moduleAny other comments?
This is a work in progress