Skip to content

Conversation

@Harold-lkk
Copy link
Contributor

@Harold-lkk Harold-lkk commented Feb 15, 2023

Copy link
Collaborator

@C1rN09 C1rN09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some comments! Also please add the api docs.

@Harold-lkk Harold-lkk requested a review from C1rN09 February 17, 2023 05:46
@Harold-lkk Harold-lkk force-pushed the lkk/OneMinusNormEditDistance branch from dda766e to b7e8395 Compare February 27, 2023 02:57
@Harold-lkk Harold-lkk force-pushed the lkk/OneMinusNormEditDistance branch from b7e8395 to 412c681 Compare February 28, 2023 07:11
@Harold-lkk Harold-lkk force-pushed the lkk/OneMinusNormEditDistance branch 2 times, most recently from 823bf0e to 1439ed3 Compare March 6, 2023 02:25
@Harold-lkk Harold-lkk requested review from C1rN09 and removed request for zhouzaida March 6, 2023 02:29
@Harold-lkk Harold-lkk force-pushed the lkk/OneMinusNormEditDistance branch from 1439ed3 to 8781329 Compare March 7, 2023 02:35
Copy link
Collaborator

@C1rN09 C1rN09 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Should rebase to main to pass CI

@Harold-lkk Harold-lkk force-pushed the lkk/OneMinusNormEditDistance branch from 8781329 to dbeb3ea Compare March 13, 2023 08:48
invalid_symbol: str = '[^A-Za-z0-9\u4e00-\u9fa5]',
**kwargs):
super().__init__(**kwargs)

Copy link
Collaborator

@zhouzaida zhouzaida Mar 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rapidfuzz is not added in runtime.txt so the metric will throw an error when calling add method. The solution is to check it in __init__. Refer to

raise ImportError(f'For availability of {self.__class__.__name__},'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants