Modify JaroWinkler boosting to match behaviour of jellyfish algorithm #76
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
|
Argh, no, please don't apply this yet. While the idea is right, there are some other tests that currently fail (namely, left="0", right="00"). I don't have time to fix it today... will get back to you soon. |
|
Deal. Thank you for working on it :) |
|
Ah, it turns out to be an issue with jellyfish; I've just reported it there: jamesturk/jellyfish#147 I'll keep this PR updated when things get resolved there. |
|
OK, jellyfish has now been updated to version 0.8.9 fixing this issue. This PR (against textdistance) now works correctly. I don't know whether there's an easy way to say that with this patch, version 0.8.9 of jellyfish is required for the JaroWinkler tests to pass. |
|
Great! Thank you. |
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.
Jellyfish has recently modified its JaroWinkler algorithm to allow for boosting even when one of the strings is shorter than 4 characters: jamesturk/jellyfish@87f9679. It is very unclear whether this is a good idea or not. But as it is, the tests now fail, as the internal and external algorithms give different results on a pair of strings such as ":" and ":0".
This patch replicates the change that jellyfish has made, which will then allow the external tests to pass once again. It also modifies the expected value of the comparison "fog" and "frog" to match this new algorithm behaviour.
If you do not wish to apply this patch, then the external tests will need modifying to exclude the case where either of the strings has length < 4.