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

Inconsistency between API docs and API implementations #40

Open
betterenvi opened this issue Oct 19, 2017 · 3 comments
Open

Inconsistency between API docs and API implementations #40

betterenvi opened this issue Oct 19, 2017 · 3 comments

Comments

@betterenvi
Copy link
Contributor

betterenvi commented Oct 19, 2017

In search_knn and search_nn, the APIs have an arg named dist, and the API docs say

dist is a distance function, expecting two points and returning distance value. Distance values can be any comparable type.

But in the implementations of the two APIs, only squared Euclidean distance is considered.

@stefankoegl
Copy link
Owner

search_knn will use Euclidean distance as a default if no own dist function is provided, see

kdtree/kdtree.py

Lines 417 to 420 in 587edc7

if dist is None:
get_dist = lambda n: n.dist(point)
else:
get_dist = lambda n: dist(n.data, point)

search_nn calls search_knn and passes along its dist parameter.

Maybe I'm missing something here. Can you please elaborate?

@betterenvi
Copy link
Contributor Author

betterenvi commented Oct 21, 2017

If no own dist function is provided, everything is fine. The API will break if dist function is provided and is not squared Euclidean distance, because the implementation uses this distance.

kdtree/kdtree.py

Lines 453 to 454 in 587edc7

plane_dist = point[self.axis] - split_plane
plane_dist2 = plane_dist * plane_dist

@stefankoegl
Copy link
Owner

I have implemented a fix in #43, which should address this issue. Please let me know if this works for you.

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

No branches or pull requests

2 participants