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

make_circles always generates an even number of samples #10037

Closed
christianbraune79 opened this Issue Oct 29, 2017 · 5 comments

Comments

Projects
None yet
4 participants
@christianbraune79
Contributor

christianbraune79 commented Oct 29, 2017

Description

sklearn.datasets.make_circles always generates an even number of samples, even when specifically asked for an uneven number.

Steps/Code to Reproduce

Example:

from sklearn.datasets import make_circles

n_samples = 101
xs, ls = make_circles(n_samples=n_samples)
assert(len(xs)==len(ls)==n_samples)

Expected Results

No Assertion Error should be thrown (in the example) OR the amount of data points generated should match the value supplied to the function OR a warning should be given.

Possible fix: Calculate the number of data points per cluster as in make_moons

Actual Results

Assertion Error, No warning, lower number of points returned than asked for.

Versions

Windows-10-10.0.15063
('Python', '2.7.11 (v2.7.11:6d1b6a68f775, Dec 5 2015, 20:32:19) [MSC v.1500 32 bit (Intel)]')
('NumPy', '1.13.0')
('SciPy', '0.16.1')
('Scikit-Learn', '0.19.1')

christianbraune79 pushed a commit to christianbraune79/scikit-learn that referenced this issue Oct 29, 2017

@rth

This comment has been minimized.

Show comment
Hide comment
@rth

rth Oct 30, 2017

Member

Thanks for opening the issue. I can confirm this. Also make_circles doesn't seem to have any specific unit tests. It is always called with an even n_samples in the existing scikit-learn code base.

A pull request to fix this issue (and add some unit tests) would be welcome..

Member

rth commented Oct 30, 2017

Thanks for opening the issue. I can confirm this. Also make_circles doesn't seem to have any specific unit tests. It is always called with an even n_samples in the existing scikit-learn code base.

A pull request to fix this issue (and add some unit tests) would be welcome..

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Oct 30, 2017

Member

My bad!

Member

amueller commented Oct 30, 2017

My bad!

@christianbraune79

This comment has been minimized.

Show comment
Hide comment
@christianbraune79

christianbraune79 Oct 31, 2017

Contributor

@rth Where would I place the unit test? In one of the existing files? Or should I open a new test file for dataset generation?

Contributor

christianbraune79 commented Oct 31, 2017

@rth Where would I place the unit test? In one of the existing files? Or should I open a new test file for dataset generation?

@jnothman

This comment has been minimized.

Show comment
Hide comment
@jnothman

jnothman Oct 31, 2017

Member
Member

jnothman commented Oct 31, 2017

@christianbraune79

This comment has been minimized.

Show comment
Hide comment
@christianbraune79

christianbraune79 Oct 31, 2017

Contributor

Ah, right. I'll add the tests and add them to the commit/pull request.

Contributor

christianbraune79 commented Oct 31, 2017

Ah, right. I'll add the tests and add them to the commit/pull request.

christianbraune79 pushed a commit to christianbraune79/scikit-learn that referenced this issue Oct 31, 2017

Christian Braune
tests for #10037, tests for odd number of samples and whether generat…
…ed points lie on the expected circles similar to test_make_moons()

@jnothman jnothman closed this in #10045 Nov 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment