-
Notifications
You must be signed in to change notification settings - Fork 178
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
Exposes views on the HDBSCAN cluster exemplars #229
Conversation
@geoffreydstewart would you mind looking over this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good. I'm not sure if it's worth adding some minimal test coverage for the new getClusters
method. The logic looks quite safe, but it could be instructive for the community. Additionally, there is an existing test deserializeHdbscanModelV42Test
where we might consider asserting that calling the getMaxDistToEdge
method on a ClusterExemplar
from a v4.2 model returns Double.NEGATIVE_INFINITY
.
I've expanded the tests to cover those points. |
The added coverage looks great. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Description
Adds a get method for cluster exemplars in HDBSCAN which returns copies of the exemplars (as the vector is mutable), along with a method that returns the exemplar in terms of feature names.
Motivation
Fixes #217.