Skip to content

DOC versionadded randomized_svd #5512

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 21, 2015
Merged

DOC versionadded randomized_svd #5512

merged 1 commit into from
Oct 21, 2015

Conversation

giorgiop
Copy link
Contributor

#5141/comment addressed.

@giorgiop
Copy link
Contributor Author

ping @amueller

@amueller
Copy link
Member

Do we also want to say what changed? I.e. what the defaults were before? ping @ogrisel @GaelVaroquaux

@amueller
Copy link
Member

yeah and please check the rendering as I think @GaelVaroquaux just said IRL

@giorgiop
Copy link
Contributor Author

Do we also want to say what changed? I.e. what the defaults were before?

I would rather keep the docstring clean from this. One can always navigate previous version to check.

@giorgiop giorgiop closed this Oct 21, 2015
@giorgiop
Copy link
Contributor Author

Closed by mistake

@giorgiop giorgiop reopened this Oct 21, 2015
@giorgiop giorgiop force-pushed the randomized-svd-versionadded branch from 2a4d1d8 to 5f24df4 Compare October 21, 2015 14:53
KamalakerDadi
KamalakerDadi reviewed Oct 21, 2015
View reviewed changes
sklearn/decomposition/pca.py
@@ -488,7 +488,8 @@ class RandomizedPCA(BaseEstimator, TransformerMixin):
use fit_transform(X) instead.

iterated_power : int, optional
Number of iterations for the power method. 3 by default.
Number of iterations for the power method. 2 by default.
.. versionchanged:: 0.18
Copy link
Contributor

Choose a reason for hiding this comment

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

I think what @amueller @ogrisel suggested was to add before the parameter. You could check.

Copy link
Member

Choose a reason for hiding this comment

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

no this is good. I'm not sure if it needs a newline above it to render correctly. can you check please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's indeed the case. Fixed now.

screen shot 2015-10-21 at 6 33 19 pm

Copy link
Member

Choose a reason for hiding this comment

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

thanks for checking. It doesn't look to be fixed here, though. maybe you didn't push?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now it is :)

@giorgiop giorgiop force-pushed the randomized-svd-versionadded branch 2 times, most recently from f8ed25b to 1872037 Compare October 21, 2015 15:24
@giorgiop giorgiop force-pushed the randomized-svd-versionadded branch from 1872037 to f45f260 Compare October 21, 2015 16:44
amueller added a commit that referenced this pull request Oct 21, 2015
@amueller
Merge pull request #5512 from giorgiop/randomized-svd-versionadded
3c988d5 
DOC versionadded randomized_svd
@amueller amueller merged commit 3c988d5 into scikit-learn:master Oct 21, 2015
@amueller
Copy link
Member

merging as minor. Thanks @giorgiop

@giorgiop giorgiop deleted the randomized-svd-versionadded branch November 3, 2015 12:28
@giorgiop giorgiop restored the randomized-svd-versionadded branch February 21, 2016 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@giorgiop @amueller @KamalakerDadi