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.

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

Tests for #53 #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Tests for #53 #56

wants to merge 1 commit into from

Conversation

bvacaliuc
Copy link
Contributor

As requested, added tests for PR for #53

@coveralls
Copy link

coveralls commented Jun 29, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.4%) to 79.609% when pulling c756cfe on bvacaliuc:tests_53 into ca13028 on sethoscope:master.

@sethoscope
Copy link
Owner

Hi, Bogdan. Thanks for adding the weight handling. Bonus points for writing a test case!
I notice the test data contains 2590 points. It seems like you could test this well with just three points (one negative, one small, one large), and it would be much easier to understand. As it is, I can't tell whether the test would fail if weights were ignored.

Any interest in trying that? If not, I can aim to get to it eventually.

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.

None yet
3 participants
@bvacaliuc @coveralls @sethoscope