Skip to content

[MRG+1] Nearest neighbor radius query should include points in the boundary.#3919

Closed
tvarvadoukas wants to merge 3 commits intoscikit-learn:masterfrom
tvarvadoukas:NearestNeighborRadiusBug
Closed

[MRG+1] Nearest neighbor radius query should include points in the boundary.#3919
tvarvadoukas wants to merge 3 commits intoscikit-learn:masterfrom
tvarvadoukas:NearestNeighborRadiusBug

Conversation

@tvarvadoukas
Copy link
Copy Markdown

Noticed that although kd_tree and ball_tree algorithms return points in the boundaries, the brute method does not. Included a fix for brute and added a toy testcase to check the consistency between the algorithms for this edgecase.

tr0n1c added 2 commits November 30, 2014 13:45
…he boundary of a radius search in the 'brute' method.
… algorithms. They should all return the same result and include the points in the boundaries.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please install pep8, run it on the files impacted by this PR and fix the warnings related only to your change.

@ogrisel
Copy link
Copy Markdown
Member

ogrisel commented Nov 30, 2014

LGTM, thanks @tvarvadoukas!

@ogrisel ogrisel changed the title [MRG] Nearest neighbor radius query should include points in the boundary. [MRG+1] Nearest neighbor radius query should include points in the boundary. Nov 30, 2014
@ogrisel
Copy link
Copy Markdown
Member

ogrisel commented Dec 2, 2014

@jnothman this is related to your PR on refactoring neighbor estimators (although should be mergeable independently).

@amueller
Copy link
Copy Markdown
Member

amueller commented Dec 2, 2014

LGTM.

@amueller amueller closed this Dec 2, 2014
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.

3 participants