Fix Editex overlapping letter groups (fixes #77) - #106
Open
youdie006 wants to merge 1 commit into
Open
Conversation
EditexHelper.letter_groups was a flat char->int dict. The canonical Editex
letter groups (Zobel & Dart 1996) overlap: C is in both {C,K,Q} and {C,S,Z},
P is in both {B,P} and {F,P,V}, and S/Z are in both {S,X,Z} and {C,S,Z}. A
one-to-one char->group mapping cannot represent a character in more than one
group, so later dict writes clobbered earlier keys (e.g. P was moved from
{B,P} to {F,P,V}). The last line also had a typo, {C,S,J} instead of the
canonical {C,S,Z}, so Z was never in group 9 - the exact case in anhaidgroup#77. As a
result r_cost's single-group equality test missed shared-group pairs and
returned mismatch_cost (2) instead of group_cost (1).
Store the 10 canonical groups as a tuple of frozensets (matching the abydos
reference cited in the module) and have r_cost return the group cost when the
two characters share any group. d_cost/silent-letter handling and all
non-overlapping return values are unchanged.
Fixes anhaidgroup#77.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #77.
Problem
EditexHelper.letter_groupswas a flatchar -> intdict. The canonical Editex letter groups (Zobel & Dart 1996) OVERLAP -- C is in both {C,K,Q} and {C,S,Z}, P is in both {B,P} and {F,P,V}, and S/Z are in both {S,X,Z} and {C,S,Z}. A one-to-one char->group mapping cannot represent a character that belongs to more than one group, so later dict writes clobbered earlier keys (e.g.letter_groups['P']=1was overwritten by...=7). As a resultr_cost's single-group equality test missed shared-group pairs and returned the mismatch cost (2) instead of the group cost (1). The last line also had a typo,{C,S,J}instead of the canonical{C,S,Z}, so Z was never in group 9 -- exactly the case reported in #77.Fix
Replace the flat dict with the 10 canonical overlapping groups as a tuple of frozensets (the abydos reference already cited in the module docstring: AEIOUY, BP, CKQ, DT, LR, MN, GJ, FPV, SXZ, CSZ), and have
r_costreturn the group cost when the two characters share ANY group.d_cost/silent-letter handling and all non-overlapping return values are unchanged.Tests
Added
test_editex_overlapping_groupsasserting the shared-group cases now cost 1, not 2:Red/green verified in a clean python:3.12 container: the new test fails (2 != 1) before the fix and passes after; all pre-existing Editex tests and the
get_raw_scoredocstring examples continue to pass unchanged (zero regression -- no existing test exercises any of the pairs whose cost changes).Reference: Zobel, J. & Dart, P. (1996), "Phonetic string matching: Lessons from information retrieval"; abydos distance implementation.
This change was prepared with AI assistance and reviewed by me before submission.