fuzzywuzzy icon indicating copy to clipboard operation
fuzzywuzzy copied to clipboard

Unit test and fixed bug

Open kketernality opened this issue 5 years ago • 4 comments

This pull request contains the following updates:

Unit Test

  1. Implemented unit test using Catch2 [1] and ported suitable tests from the original Python repository [2]. The unit test files locate in the test folder as before.
  2. Updated CMakeLists.txt and README.md to provide instruction of building and running unit test.

Fixed Bugs

  • [v] The example from Readme gave the wrong result. The cause is that the full_process is default on and will strip the tailing exclamation mark. It was fixed by altering the default value of full_process to false.
ReadmeExample
FAILED:
      REQUIRE( 97 == fuzz::ratio("this is a test", "this is a test!") );
EXPANSION:
      97 == 100 
  • [v] Comparison of two empty strings should give 100% similarity as in the original Python implementation. Fixed by adding additional logic in ratio() and partial_ratio().
RatioTest
      testEmptyStringsScore100
FAILED:
      REQUIRE( 100 == fuzz::ratio("", "") )
EXPANSION:
      100 == 0
  • [v] The partial_ratio() of two identical string is 0% similarity which should be 100% undoubtedly. The detailed description is in the fuzzywuzzy.cpp. This was fixed by a temporary solution to compare if two strings is identical or not.
RatioTest
      testPartialRatio
FAILED:
      REQUIRE( 100 == fuzz::partial_ratio("new york mets", "new york mets") )
EXPANSION:
      100 == 0

kketernality avatar Mar 08 '20 08:03 kketernality

Thanks for taking the time to port unit tests over! I'll see about going over the commits the coming week.

tmplt avatar Mar 12 '20 12:03 tmplt

Just let me know if there is anything goes wrong or needs to be updated. ^_^

kketernality avatar Mar 13 '20 04:03 kketernality

I'm not partial to adding a ~17kloc library header. Could you source catch.hpp from a submodule instead?

Okay, I think make it as an installed package or something likewise is more proper. Will be added in the next commit.

kketernality avatar Apr 02 '20 08:04 kketernality

@tmplt

I had updated the required change. Please review the code changes in this new commit.

kketernality avatar May 12 '20 02:05 kketernality