Knn refactor #54

Merged
merged 3 commits into from Jul 27, 2014

Conversation

Projects
None yet
3 participants
@njern
Collaborator

njern commented Jul 20, 2014

I noticed that whether the user picked "euclidean" or "manhattan", both switch statement were still calling - pairwiseMetrics.NewEuclidean()

Fixed the bug and refactored the code to make it a bit shorter at the same time.

njern added some commits Jul 20, 2014

@sjwhitworth

This comment has been minimized.

Show comment
Hide comment
@sjwhitworth

sjwhitworth Jul 20, 2014

Owner

Thanks! Looks good. However, Travis doesn't seem to like the build for some reason - can you investigate?

Owner

sjwhitworth commented Jul 20, 2014

Thanks! Looks good. However, Travis doesn't seem to like the build for some reason - can you investigate?

@njern

This comment has been minimized.

Show comment
Hide comment
@njern

njern Jul 20, 2014

Collaborator

I need to set up a pre-commit hook to run go test. I'd gone and done "just a small change" between the time I ran the tests and when I committed and of course it broke things :)

Collaborator

njern commented Jul 20, 2014

I need to set up a pre-commit hook to run go test. I'd gone and done "just a small change" between the time I ran the tests and when I committed and of course it broke things :)

@lazywei

This comment has been minimized.

Show comment
Hide comment
@lazywei

lazywei Jul 23, 2014

Collaborator

Looks good to me!
LGTM

Collaborator

lazywei commented Jul 23, 2014

Looks good to me!
LGTM

sjwhitworth added a commit that referenced this pull request Jul 27, 2014

@sjwhitworth sjwhitworth merged commit e6c28ef into sjwhitworth:master Jul 27, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment