Skip to content
This repository has been archived by the owner on Aug 26, 2024. It is now read-only.

fuzzywuzzy returns 0 instead of 100 in many corner cases #196

Open
rralf opened this issue Mar 21, 2018 · 9 comments
Open

fuzzywuzzy returns 0 instead of 100 in many corner cases #196

rralf opened this issue Mar 21, 2018 · 9 comments

Comments

@rralf
Copy link
Contributor

rralf commented Mar 21, 2018

Hi,

following issue(s) can be reproduced with python-levenshtein 0.12.0 and fuzzywuzzy 0.16.0

There are several combinations that should be considered similar while being scored as absolutely dissimilar.

fuzz.ratio('', '')
evaluates to 0. Two empty absolutely identical, so this should be 100.

fuzz.ratio('{', '{')
evaluates to 100, while
fuzz.token_sort_ratio(['{'], ['{'])
evaluates erroneously to 0. Inconsistently,
fuzz.token_sort_ratio(['{a'], ['{a'])
evaluates to 100. (which is correct IMO).

Proposal: Independent of the method, fuzzywuzzy should always return 100 if both arguments are absolutely equal. I think this could efficiently be checked.

Thanks!

@josegonzalez
Copy link
Contributor

Pull requests - with the accompanying tests - are welcome and encouraged :)

@rralf
Copy link
Contributor Author

rralf commented Mar 21, 2018

First I'd like to discuss those issues, I didn't have a look into the code yet. Additionally I still have to check if this behaviour depends on python-levenshtein (which I'm using) or not.

Maybe there's even some rationale behind this and the behaviour is intended for some reason.

Or in other words: Am I facing a bug or does it work as intended? :-)

@josegonzalez
Copy link
Contributor

From my perspective, the tests pass, so any behavior is expected. If you can add this functionality and tests continue to pass, great!

One thing to note is that the ratio may be misleading if the tokens are all the same but actually mis-ordered. That might have actual implications on someone's code, but if we're not testing for that now, its fine to break imo.

@rralf
Copy link
Contributor Author

rralf commented Mar 22, 2018

I'm currently writing some patches.

Think I will have to touch the check_for_none and check_empty_string decorators... Let me return with a pull request in a couple of hours.

@josegonzalez
Copy link
Contributor

Please be sure to include perf benchmarks :)

@rralf
Copy link
Contributor Author

rralf commented Mar 22, 2018

Hi,

before requesting to pull, here are the fixes.

How can I run performance benchmarks?

Thanks

@zackkitzmiller
Copy link
Contributor

@rralf I looked over those changes and can assure @josegonzalez that they are performant. @josegonzalez do you actually need a perf tests here? This changeset looks good to me.

@josegonzalez
Copy link
Contributor

Sounds fine with me. PR it.

@rralf
Copy link
Contributor Author

rralf commented Mar 22, 2018

Huh, that went quickly... Thanks for reviewing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants