-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
doc: Standarize default documentation for feature_selection #17465
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
doc: Standarize default documentation for feature_selection #17465
Conversation
Thanks for the PR! Make sure you check the linting failure:
|
- Change `default` documentation of parameters to follow `default=<value>` template
c6cd65b
to
3af9b04
Compare
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 @kohakukun , mostly looks good
I made a few comments, some of them are also relevant for other parts in the PR so make sure to check them all ;)
@@ -73,7 +73,7 @@ class SelectFromModel(MetaEstimatorMixin, SelectorMixin, BaseEstimator): | |||
or a non-fitted estimator. The estimator must have either a | |||
``feature_importances_`` or ``coef_`` attribute after fitting. | |||
|
|||
threshold : string, float, optional default None | |||
threshold : string, float, default=None |
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.
threshold : string, float, default=None | |
threshold : string or float, default=None |
sklearn/feature_selection/_base.py
Outdated
@@ -32,7 +32,7 @@ def get_support(self, indices=False): | |||
|
|||
Parameters | |||
---------- | |||
indices : boolean (default False) | |||
indices : boolean, default=False |
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.
indices : boolean, default=False | |
indices : bool, default=False |
@@ -198,7 +198,7 @@ def fit(self, X, y=None, **fit_params): | |||
X : array-like of shape (n_samples, n_features) | |||
The training input samples. | |||
|
|||
y : array-like, shape (n_samples,) | |||
y : array-like, shape (n_samples,), default=None |
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.
y : array-like, shape (n_samples,), default=None | |
y : array-like of shape (n_samples,), default=None |
@@ -586,11 +589,13 @@ class SelectFpr(_BaseFilter): | |||
mutual_info_classif: | |||
f_regression: F-value between label/feature for regression tasks. | |||
mutual_info_regression: Mutual information between features and the target. | |||
SelectPercentile: Select features based on percentile of the highest scores. | |||
SelectPercentile: Select features based on percentile of the highest | |||
scores. |
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 think you'll need to indend these to the right a little bit
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 @kohakukun
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.
Looks good to me.
…arn#17465) * doc: Standarize default documentation for feature_selection - Change `default` documentation of parameters to follow `default=<value>` template * remove space at end-of-line * fix linting issues * address comments * missing file
…arn#17465) * doc: Standarize default documentation for feature_selection - Change `default` documentation of parameters to follow `default=<value>` template * remove space at end-of-line * fix linting issues * address comments * missing file
Reference Issues/PRs
Fixes small section of #15761
What does this implement/fix? Explain your changes.
default
documentation of parameters to followdefault=<value>
template
Any other comments?
#DataUmbrella