8000 doc: Standarize default documentation for feature_selection by kohakukun · Pull Request #17465 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 5 commits into from
Jun 7, 2020

Conversation

kohakukun
Copy link
Contributor
@kohakukun kohakukun commented Jun 6, 2020

Reference Issues/PRs

Fixes small section of #15761

What does this implement/fix? Explain your changes.

  • Change default documentation of parameters to follow default=<value>
    template

Any other comments?

#DataUmbrella

@amueller
Copy link
Member
amueller commented Jun 6, 2020

Thanks for the PR! Make sure you check the linting failure:

sklearn/feature_selection/_from_model.py:75:1: W293 blank line contains whitespace

kohakukun added 3 commits June 6, 2020 18:47
- Change `default` documentation of parameters to follow `default=<value>`
template
@kohakukun kohakukun force-pushed the doc/feature_selection branch from c6cd65b to 3af9b04 Compare June 6, 2020 16:48
Copy link
Member
@NicolasHug NicolasHug left a 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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
threshold : string, float, default=None
threshold : string or float, default=None

@@ -32,7 +32,7 @@ def get_support(self, indices=False):

Parameters
----------
indices : boolean (default False)
indices : boolean, default=False
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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.
Copy link
Member

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

Copy link
Member
@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

thanks @kohakukun

Copy link
Member
@amueller amueller left a 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.

@adrinjalali adrinjalali merged commit 5297365 into scikit-learn:master Jun 7, 2020
@kohakukun kohakukun deleted the doc/feature_selection branch June 7, 2020 18:00
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
…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
jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
…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
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.

4 participants
0