Skip to content

refactor: improve naming, structure, and documentation with regression proof - #253

Merged
dmort27 merged 1 commit into
dmort27:masterfrom
Wolfvin:refactor/naming-and-structure
Jun 15, 2026
Merged

refactor: improve naming, structure, and documentation with regression proof#253
dmort27 merged 1 commit into
dmort27:masterfrom
Wolfvin:refactor/naming-and-structure

Conversation

@Wolfvin

@Wolfvin Wolfvin commented Jun 13, 2026

Copy link
Copy Markdown

Summary

This PR improves naming clarity, structural organization, and documentation across core epitran modules, with full behavioral regression proof via Regrets.

All 10 behavioral clusters GREEN, all 5 chain tests MATCH, all 40 outputs identical to pre-refactor baseline.

Naming Improvements

Old Name New Name Rationale
_non_deterministic_mappings _find_ambiguous_mappings Finds one-to-many grapheme-phoneme mappings
_load_g2p_map _load_grapheme_to_phoneme_map Self-documenting; no more opaque g2p abbreviation
_construct_regex _build_greedy_match_regex Explains maximal munch tokenization strategy
is_korean contains_korean_syllables Precise: detects Hangul syllable characters
ligaturize convert_affricates_to_ligatures Self-explanatory (alias preserved)
StripDiacritics.process strip_specified_diacritics No more generic process (alias preserved)
_fields_to_function _compile_replacement_rule Describes what is compiled
_fields_to_function_metathesis _compile_metathesis_rule Same pattern
_sub_symbols _expand_symbol_references Describes transformation

Structural Improvements

  • Extract AFFRICATE_LIGATURES module-level constant in ligaturize.py — mapping data separated from function logic
  • Extract SPECIAL_LANGUAGE_BACKENDS module-level constant in _epitran.py — special backends dict was buried inside class; now discoverable
  • Add epitran/adapters/ module — thin wrapper modules for regression testing of class-based APIs

Backward Compatibility

All renamed public methods retain their old names as aliases:

  • ligaturize = convert_affricates_to_ligatures
  • StripDiacritics.process = strip_specified_diacritics
  • SimpleEpitran.is_korean = contains_korean_syllables
  • Epitran.special = SPECIAL_LANGUAGE_BACKENDS

Regression Proof

Tested with Regrets — output-based regression testing:

VERIFICATION 1 — Regrets Fingerprint

All 10 clusters GREEN after refactor.

VERIFICATION 2 — Raw Output vs Pre-Refactor Baseline

All 40 outputs IDENTICAL to pre-refactor baseline.

VERIFICATION 3 — Fingerprint Cross-Check

All 10 fingerprints match pre-refactor truth.

VERIFICATION 4 — Chain Validation

All 5 chains MATCH pre-refactor chain hashes.

Fingerprint Before/After

Cluster Before After Match
spa-transliterate 5x4d98i 5x4d98i
deu-transliterate 5j37svh 5j37svh
fra-transliterate 3f8syig 3f8syig
spa-strict-trans 5x4d98i 5x4d98i
spa-word-to-tuples 6724o1o 6724o1o
ligaturize 5pxpkk6 5pxpkk6
puncnorm 1z6dwyb 1z6dwyb
strip-diacritics 2daij2n 2daij2n
ipa-to-xsampa 5u5264e 5u5264e
rules-apply 4mcbm7s 4mcbm7s

Chain Hashes Before/After

Chain Before After Match
spanish-to-xsampa 389p9re 389p9re
spanish-to-ligatures 6w1sy7s 6w1sy7s
french-pipeline 4ue8mxi 4ue8mxi
german-pipeline 9ljbs80 9ljbs80
preprocess-transliterate-postprocess 2slbp3y 2slbp3y

Structural refactoring with full regression proof via Regrets.
All 10 behavioral clusters GREEN, all 5 chains MATCH, all 40 outputs
identical to pre-refactor baseline.

## Naming Improvements

- SimpleEpitran._non_deterministic_mappings → _find_ambiguous_mappings
  (clearer: finds one-to-many grapheme-phoneme mappings)

- SimpleEpitran._load_g2p_map → _load_grapheme_to_phoneme_map
  (self-documenting: no more opaque 'g2p' abbreviation)

- SimpleEpitran._construct_regex → _build_greedy_match_regex
  (explains maximal munch tokenization strategy)

- SimpleEpitran.is_korean → contains_korean_syllables
  (precise: detects Hangul syllable characters, not 'Korean-ness')

- ligaturize → convert_affricates_to_ligatures
  (self-explanatory name; backward-compatible alias preserved)

- StripDiacritics.process → strip_specified_diacritics
  (no more generic 'process'; backward-compatible alias preserved)

- Rules._fields_to_function → _compile_replacement_rule
  (describes what is compiled: a context-sensitive replacement rule)

- Rules._fields_to_function_metathesis → _compile_metathesis_rule
  (same pattern: describes the rule type being compiled)

- Rules._sub_symbols → _expand_symbol_references
  (describes transformation: expanding ::symbol:: references)

## Structural Improvements

- Extract AFFRICATE_LIGATURES module-level constant in ligaturize.py
  (mapping data separated from function logic)

- Extract SPECIAL_LANGUAGE_BACKENDS module-level constant in _epitran.py
  (special backends dict was buried inside class; now discoverable)

- Add epitran/adapters/ module for Regrets regression testing
  (thin wrapper modules exposing instance methods as standalone functions)

## Documentation

- Added docstrings to renamed methods explaining their purpose
- Added JSDoc-style descriptions for extracted constants

## Backward Compatibility

All renamed public methods retain their old names as aliases:
- ligaturize = convert_affricates_to_ligatures
- StripDiacritics.process = strip_specified_diacritics
- SimpleEpitran.is_korean = contains_korean_syllables
- Epitran.special = SPECIAL_LANGUAGE_BACKENDS

## Regression Proof (via Regrets)

KEBENARAN 1 (pre-refactor raw output, 40 test cases):
All outputs IDENTICAL after refactoring.

KEBENARAN 2 (pre-refactor fingerprints + chain hashes):
All 10 fingerprints MATCH. All 5 chain hashes MATCH.

| Cluster | Before | After | Match |
|---------|--------|-------|-------|
| spa-transliterate | 5x4d98i | 5x4d98i | ✅ |
| deu-transliterate | 5j37svh | 5j37svh | ✅ |
| fra-transliterate | 3f8syig | 3f8syig | ✅ |
| spa-strict-trans | 5x4d98i | 5x4d98i | ✅ |
| spa-word-to-tuples | 6724o1o | 6724o1o | ✅ |
| ligaturize | 5pxpkk6 | 5pxpkk6 | ✅ |
| puncnorm | 1z6dwyb | 1z6dwyb | ✅ |
| strip-diacritics | 2daij2n | 2daij2n | ✅ |
| ipa-to-xsampa | 5u5264e | 5u5264e | ✅ |
| rules-apply | 4mcbm7s | 4mcbm7s | ✅ |

Chain hashes (before → after):
| Chain | Before | After | Match |
|-------|--------|-------|-------|
| spanish-to-xsampa | 389p9re | 389p9re | ✅ |
| spanish-to-ligatures | 6w1sy7s | 6w1sy7s | ✅ |
| french-pipeline | 4ue8mxi | 4ue8mxi | ✅ |
| german-pipeline | 9ljbs80 | 9ljbs80 | ✅ |
| preprocess-transliterate-postprocess | 2slbp3y | 2slbp3y | ✅ |

@dmort27 dmort27 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good.

@dmort27
dmort27 merged commit cfd20cc into dmort27:master Jun 15, 2026
3 checks passed
@juice500ml

Copy link
Copy Markdown
Collaborator

Why are adapters necessary here? The fingerprints (I'm not sure what this is too) doesn't cover numerous languages that epitran covers too. If not being reasoned more better than this, I prefer to revert this commit, especially epitran/adapters. @Wolfvin and @dmort27 , can y'all weigh in on this? Thanks!

@juice500ml juice500ml self-assigned this Jun 16, 2026
@juice500ml

Copy link
Copy Markdown
Collaborator

FYI epitran/adapters have been removed & shipped at #254, please raise a new issue if such folder is necessary; we can discuss more.

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