Initialize indices with 1, allows for Infs and NaNs in the data
#126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tried out NaN on data, and turns out in that case we still have problems. Hard to figure out what to do in that case, will need more work, not as easy as this patch. Left a commented test for that issue. |
axsk
reviewed
Oct 25, 2021
| @@ -31,6 +16,21 @@ end | |||
| end | |||
| end | |||
|
|
|||
| # # Test for issue #78 | |||
There was a problem hiding this comment.
Not sure why you commented this out,
but if its merely because the test fails, a @test_broken might be appropriate so we know its broken :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The patch merely modifies the algorithm to initialize the index list with
1instead of-1. New tests are added that reproduce issues #78 and #125.It's a bit of a decision what to do with NaNs, in my opinion it's best if we ensure indices are always valid, and a NaN distance is as good as Inf.