Repository navigation
Detect the language of text made of multi-word dictionary entries - #1435
AdrianAtZyte wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1435 +/- ##
==========================================
- Coverage 99.09% 99.05% -0.05%
==========================================
Files 239 239
Lines 3988 4011 +23
==========================================
+ Hits 3952 3973 +21
- Misses 36 38 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
serhii73
left a comment
There was a problem hiding this comment.
Code review (claude code-review, at 2b73aca; compared with the PR's merge-base because the branch is 56 commits behind master). Not approving as is:
- Scores are inflated per word. A multi-word entry now counts once per word, and every locale that has those words as separate single-word entries gets the same inflated score, while locales are ranked by that count first.
detect_language('I will see you next week at noon', languages=None)returnsffon this head (nlat the merge-base;enis the right answer and scores [2,3] against ff's [3,0]), andsearch_dateson that text with auto-detection returns the junk match('see', ...)plus a misdatednext week.count_applicability('next week')goes from [0,0] to [2,0], andMeet me on 5 June next weektiesnnanddjewithen. - Deduplication changed. The set of individual words replaces
set(words)over whole tokens, so tokens that share a word undercount (last weekandnext weekcountweekonce), and scores depend on which phrases happen to share words. - An existing expectation was changed. The Filipino sentence in
tests/test_search.pyflips fromtltofilbecause the inflated scores change which tied locale wins. That edit hides a behaviour change instead of showing it is intended. - Tests are too narrow. They only check text that is a single multi-word entry (
next week,next month). They do not cover mixed text, repeated entries, or several competing languages, so theffresult above passes the suite.
The branch also needs a rebase; #1401 changes _count_words_present_in_the_dictionary too (a guard so padded chunks don't credit parts), so this PR may overlap with it. Counting a multi-word entry once, when all of its words are present, would keep scores comparable.
Fixes #1163