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 alt output format of slice of predictions #7

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

Conversation

RenatoGeh
Copy link

@RenatoGeh RenatoGeh commented Nov 13, 2018

This patch provides an alternative output format. Previously, retrieving
an array of predictions was worst case quadratic time, as each cluster's
observations would have to be compared with the original observations
and then assigned their designated cluster. This commit resolves this
issue by simply returning the already generated internal slice of
predictions from Partition.
---

This is a use case that I have. I need performance and to be able to preserve the original observations order, so I'm sending this as a PR. Totally understand if you don't want to merge this, as it doesn't use your muesli/clusters format.

I wasn't sure on the name (PartitionPoints), so feel free to change that (or provide suggestions for a v2).

This patch provides an alternative output format. Previously, retrieving
an array of predictions was worst case quadratic time, as each cluster's
observations would have to be compared with the original observations
and then assigned their designated cluster. This commit resolves this
issue by simply returning the already generated internal slice of
predictions from Partition.
@coveralls
Copy link

coveralls commented Nov 13, 2018

Pull Request Test Coverage Report for Build 42

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+4.03%) to 42.623%

Totals Coverage Status
Change from base Build 41: 4.03%
Covered Lines: 52
Relevant Lines: 122

💛 - Coveralls

@RenatoGeh
Copy link
Author

Hi,

I would like to know whether a merge is still on the table, as I need to make the necessary changes in my own repositories.

Regarding Travis, a quick search shows that this error is probably due to a type alias from Go's experimental image package. Type aliases were introduced in Go 1.9, and all Travis builds from 1.9+ are successful, meaning that's probably it.

Thanks,
Renato

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.

2 participants