Skip to content
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

Add test for local linear embedding #6393

Merged

Conversation

yenchenlin
Copy link
Contributor

Test the errors raised when parameter passed to LLE is invalid.

@yenchenlin yenchenlin force-pushed the add-test-for-local-linear-embedding branch from 0db27ff to 8877ffe Compare February 18, 2016 08:40
# Test the error raised when parameter passed to lle is invalid
def test_lle_init_parameters():
rng = np.random.RandomState(0)
X = np.array(list(product(np.arange(18), repeat=2)))
Copy link
Member

Choose a reason for hiding this comment

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

You can probably have a simpler X for testing parameters checking

@jakevdp
Copy link
Member

jakevdp commented Feb 18, 2016

Thanks for the contribution! I agreed with @TomDLT's comments that the tests should be simplified after that +1 for merge.

@yenchenlin yenchenlin force-pushed the add-test-for-local-linear-embedding branch from 8877ffe to d963b2f Compare February 18, 2016 19:01
@yenchenlin
Copy link
Contributor Author

Great thanks to @TomDLT and @jakevdp for reviewing the PR and giving conducive suggestions.
I've modified the code, please have a look.

@jakevdp
Copy link
Member

jakevdp commented Feb 18, 2016

Looks good to me. Thanks!

jakevdp added a commit that referenced this pull request Feb 18, 2016
…r-embedding

Add test for local linear embedding
@jakevdp jakevdp merged commit 281ac0c into scikit-learn:master Feb 18, 2016
@yenchenlin yenchenlin deleted the add-test-for-local-linear-embedding branch February 19, 2016 00:52
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.

None yet

3 participants