Skip to content

Add skip argument to search funcs #6

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

Merged
merged 2 commits into from
Jul 15, 2020
Merged

Conversation

jlumpe
Copy link
Collaborator

@jlumpe jlumpe commented Jul 12, 2020

Add skip argument to find and find_nearest, which is a callback f(i::Int)::Bool indicating whether to exclude data point with index i from the results.

Tests require #5 be merged first.

@altre
Copy link
Collaborator

altre commented Jul 13, 2020

I'm not sure if this is a good thing to include in the API. Can't you just filter the result? Or do you have a special use case?

@jlumpe
Copy link
Collaborator Author

jlumpe commented Jul 13, 2020

For find you can definitely just filter the result. find_nearest is more complicated though because you don't know ahead of time how many data points you'll have to go through before you find k that have passed the filter. It's relevant for my uses because my data points are classified and I want to find the closest k for a point outside of its assigned class. There are other common use cases as well - for example Theiler windows in time series analysis. Basically it boils down to excluding the data points you know a priori will be close to your query in order to focus on the unexpected ones.

This is implemented in other nearest-neighbor libraries, like NearestNeighbors.jl.

@jlumpe
Copy link
Collaborator Author

jlumpe commented Jul 13, 2020

Tests implemented, so should be good to merge if you think it's appropriate.

@altre altre merged commit 150e794 into JuliaNeighbors:master Jul 15, 2020
@altre
Copy link
Collaborator

altre commented Jul 15, 2020

Makes sense to me. Thanks for the PR.

@jlumpe jlumpe deleted the skip-arg branch July 15, 2020 17:54
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