Skip to content

Updated documentation for #7947 #8196

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
wants to merge 1 commit into from
Closed

Updated documentation for #7947 #8196

wants to merge 1 commit into from

Conversation

jenishah
Copy link

Reference Issue

#7947

@@ -221,7 +221,8 @@ class PCA(_BasePCA):
The estimated number of components. When n_components is set
to 'mle' or a number between 0 and 1 (with svd_solver == 'full') this
number is estimated from input data. Otherwise it equals the parameter
n_components, or n_features if n_components is None.
n_components, or n_features if n_components is None. For PCA to work
properly, keep n_samples > n_components.
Copy link
Member

Choose a reason for hiding this comment

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

this is not specific enough. What do you mean by "properly". eg Will it crash?

@@ -221,7 +221,8 @@ class PCA(_BasePCA):
The estimated number of components. When n_components is set
to 'mle' or a number between 0 and 1 (with svd_solver == 'full') this
number is estimated from input data. Otherwise it equals the parameter
n_components, or n_features if n_components is None.
n_components, or n_features if n_components is None. For PCA to work
Copy link
Contributor

Choose a reason for hiding this comment

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

i know they weren't there before, but I think double backticks around the parameter names and None would be good here?

@qinhanmin2014
Copy link
Member

@jenishah Thanks a lot for your contribution. This PR has been resolved in #8742, so we're going to close it. Hope that you can continue to contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants