Skip to content

[MRG] Limiting n_components by both n_features and n_samples instead of just n_features (Recreated PR) #8741

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 6 commits into from

Conversation

wallygauze
Copy link
Contributor

Reference Issue

Fixes #8484

What does this implement/fix? Explain your changes.

  • 'Handling n_components==None' code in _fit method
  • Extended (to include n_samples as a limit) relevant ValueError in _fit_full method
  • Extended ValueError raised for (not 1 <= n_components <= n_features) and ValueError raised for (svd_solver == 'arpack' and n_components == n_features) in _fit_truncated

Documentation changes:

n_component parameter documentation, & mentions elsewhere in parameters section

n_components_ attribute documentation

unrelated (to issue) extra: corrected documentation for explained_variance_ratio_ attribute

Any other comments?

Original pull-request: #8486

@codecov
Copy link

codecov bot commented Apr 13, 2017

Codecov Report

Merging #8741 into master will increase coverage by <.01%.
The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8741      +/-   ##
==========================================
+ Coverage   95.51%   95.52%   +<.01%     
==========================================
  Files         342      342              
  Lines       61354    61365      +11     
==========================================
+ Hits        58605    58616      +11     
  Misses       2749     2749
Impacted Files Coverage Δ
sklearn/decomposition/tests/test_pca.py 100% <100%> (ø) ⬆️
sklearn/decomposition/pca.py 94.55% <80%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d31e55...66b3402. Read the comment docs.

@wallygauze wallygauze closed this Apr 13, 2017
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.

n_components in PCA explicitly limited by n_features only
1 participant