search

seatgeek / fuzzywuzzy

Fuzzy String Matching in Python
http://chairnerd.seatgeek.com/fuzzywuzzy-fuzzy-string-matching-in-python/
GNU General Public License v2.0
9.21k stars 874 forks source link

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

Open rralf opened 6 years ago

rralf commented 6 years ago

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 commented 6 years ago

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

rralf commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Please be sure to include perf benchmarks :)

rralf commented 6 years ago

Hi,

before requesting to pull, here are the fixes.

How can I run performance benchmarks?

Thanks

zackkitzmiller commented 6 years ago

@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 commented 6 years ago

Sounds fine with me. PR it.

rralf commented 6 years ago

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