-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
DOC fix ungrammatical sentence in doc of SelectFromModel #19240
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
Conversation
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 @EdwinWenink, this is indeed clearer.
I noticed that the docstrinrg for estimator
https://scikit-learn.org/stable/modules/generated/sklearn.feature_selection.SelectFromModel.html#sklearn.feature_selection.SelectFromModel
says
The estimator must have either a feature_importances_ or coef_ attribute after fitting.
This is actually not true anymore. Would you mind updating it to
The estimator should have a feature_importances_ or coef_ attribute after fitting. Otherwise, the
importance_getter
parameter should be used.
Thanks!
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
@NicolasHug thanks for the quick reply. Good catch with the importance_getter, I hadn't noticed. I accepted your suggested change and also pushed your other recommendation. |
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 @EdwinWenink !
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 @EdwinWenink!
…n#19240) Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Reference Issues/PRs
What does this implement/fix? Explain your changes.
I noticed an ungrammatical sentence in the user documentation for
1.13.4. Feature selection using SelectFromModel
. This is a small commit with a proposed improvement that should capture the intended meaning of the sentence.Any other comments?