Skip to content

Simplify the config side's 3.0 changes (parser_for, PolicyPatch, locale packs) - #1083

Open
derek73 wants to merge 2 commits into
masterfrom
chore/simplify-config-side
Open

derek73 wants to merge 2 commits into
masterfrom
chore/simplify-config-side

Conversation

@derek73

@derek73 derek73 commented Nov 5, 2026 •

Copy link
Copy Markdown
Owner

Pass (c) of the three file-disjoint simplify passes before the 3.0.0 tag. It covers the 3.0 cycle's changes (git diff v2.8.0 origin/master) to nameparser/_parser.py, _policy.py, _locale.py and nameparser/locales/*.py. The generated given-name modules and zh.py's generated block are excluded, and no other library file is touched. It is quality only: no behavior, message, repr or pickle change.

Changes

  • cs, sk, es, es_ar: DEVIATES = deviates_by_two_family_names replaces four one-line wrappers. Their docstrings restated the shared helper's, and in cs and sk they had gone stale ("shared with the es pack", from before es_ar existed). The registry gate (test_locales.py) asks only for a callable, and test_cs_and_sk_pack_contents compares cs's, sk's and es's to the helper.
  • parser_for:
    • The per-pack language-tag check now rewraps through _rewrapped, as the patch loop already does. The text comes out the same, while applying locale 'x': <exc>, and both raise a plain ValueError.
    • The "one choice for all the packs" comment moves down to the loop it describes. It had been sitting above the strict handling.
    • The second quote of rules/D2 (applying a pack wraps its errors with the pack's code) moves from above the warning re-emission, which it doesn't describe, to the apply_patch wrap that it does. The re-emission block itself is unchanged.
  • _no_strict_language: if locales: replaces for loc in locales[:1], a loop that ran at most once. The docstring's broken line is reflowed.
  • PolicyPatch.__post_init__: a comment listed all eight scalar fields by name. It now states the rule instead (every field but the union ones, name_order and script_orders), so adding a field can't make it stale.

Guard proof

Guard Result
tools/differential/snapshot.jsonl byte-identical (sha256 1d4368c3…), not re-recorded; test_snapshot.py passes
cldr_agreement*.json, wikidata_native_agreement*.json untouched (no file under tools/ in the diff)
tests/v2/test_locales.py incl. test_non_interference_each_pack pass
reprs (test_reprs.py) pass; also every pack, parser_for(pack), parser_for(ES, PT_BR, CS), parser_for(PT, strict=True), Parser(), Policy(): repr identical to master (38 objects)
pickles each of those 38 pickled under master loads equal in the branch, and with PYTHONHASHSEED=0 the pickle bytes are identical in both trees
layering (test_layering.py) pass; no import changed
full suite 3.11 37296 passed, 29 skipped, 5 xfailed
full suite 3.15 37296 passed, 29 skipped, 5 xfailed
serial benchmarks (-n0 -m serial) 54 passed (±2% frame band held; no parse-path code touched)
mypy, ruff check clean

Considered and left alone

  • Moving _locale._letter_runs onto _policy._script_matcher (reuse review). It would change what DEVIATES declares: halfwidth katakana counts as one run today, and hangul jamo as two. That is a behavior change, so it's out of scope here.
  • A language_pack(code, tag) helper for the nine vocabulary-only packs (reuse review). It would save about three lines per pack, but each pack would stop being a plain Locale(...) literal, the form docs/locales.rst teaches contributors to write.
  • Building the joined lexicon once instead of once per pack in _with_packs (efficiency review). It saves under 1 ms per pack at construction, and nothing on the parse path.
  • _rewrapped discarding the exception subclass, and _failed_layer's error-path rebuild (altitude review). Both sit at the right depth. An add_note rewrite would change how the message renders.
  • A finding outside the changes: Policy's repr prints segment_scripts in frozenset iteration order, which varies across processes (parser_for(locales.JA)'s repr differs between runs). It predates this PR; this is a note, not a fix.

Behavior-changing candidates (not applied)

These simpler forms were raised and refused because each changes what something declares or raises. Corpus counts are over the 6,731 distinct names in tools/differential/corpus*.jsonl (measured 2026-11-05 on this branch).

  1. _locale._letter_runs tests CJK with _policy._script_matcher(*_SCRIPT_RANGES) instead of unicodedata.name(ch).startswith(_CJK) (reuse review). One definition of "CJK" replaces two.
    • What it moves: 26 letters in the corpora are classed differently (the conjoining hangul jamo U+1100–11FF, the iteration mark 々, and others). deviates_by_two_family_names flips for 4 of the 6,731 names.
    • Gains a declaration (3): the halfwidth-katakana names ヤマダ タロウ, タナカ. John and タロウ·ヤマダ, which _SCRIPT_RANGES counts as katakana.
    • Loses a declaration (1): 김. 민준 written in conjoining jamo, which _SCRIPT_RANGES' precomposed HANGUL span doesn't cover. This is the unsafe direction for the non-interference gate, so the rewrite would first need _SCRIPT_RANGES widened to the jamo block, which is itself a behavior change for every script-conditional rule.
  2. _parser._rewrapped keeps the original exception and adds the pack's name with exc.add_note(...) instead of building a new plain TypeError/ValueError (altitude review).
    • What it moves: every wrapped construction error from parser_for's three rewrap sites. The raised type becomes the original one (a _lexicon._MaskValueError subclass would surface as itself), and "while applying locale 'x'" moves from the message's first line to a note printed under the traceback. str(exc) loses the prefix.
    • Not measured over a corpus: these are construction-time errors, not parses. The tests matching on while applying locale would all need to read __notes__.
  3. deviates_by_two_family_names counts the name tokens of a parse instead of whitespace pieces and letter runs (altitude review). This would drop the hand copy of tokenizer and segmentation knowledge (rules/T1–T3, W1).
    • What it moves (sketch: ≥3 GIVEN/MIDDLE/FAMILY tokens in a default parse()): 1,457 of the 6,731 names flip, every one from declared to undeclared, and none the other way.
    • Examples: Signor Marco Rossi, Dumas VI, Jr., John Smith (PhD, MBA), j nee Smith, কিশোৰ.
    • Losing declarations is the gate's unsafe direction. A real version would have to count under the pack's own parse, which a DEVIATES predicate cannot reach without importing the parser, a layering change.

Reviews

  • Simplify pass (four reviewers: reuse, simplification, efficiency, altitude): the findings applied are the list under Changes. Those refused for being behavior changes are listed below, and the rest are under "Considered and left alone".
  • Behavior equivalence: no non-equivalences. _reading_index.selected raises only plain ValueError at the per-pack check, so _rewrapped gives the same type, text and __cause__. if locales: is the same as the old one-pass loop. Nothing reads DEVIATES.__name__ or __doc__. This reviewer had no shell and checked by reading; the guard runs above are the executed proof.
  • Silent failures: none. No raise or warning was lost, reworded, retyped or unchained, and the catch_warnings(record=True) re-emission block is byte-identical.
  • Comments: one real error, in my new PolicyPatch comment, which called every remaining scalar a bool. unlisted_caps_suffixes is a CapsSuffixes, validated by its own check rather than a bool check. Fixed in da22a5f. The other changes were confirmed: the moved comments now sit on the code they describe (the D2 quote's old spot sat over Parser(...) construction, which is not wrapped), and the deleted DEVIATES docstrings' facts survive in _locale.deviates_by_two_family_names. The reviewer also noted that pack modules do not render through autodoc (docs/modules.rst lists only get, available).
  • Second round on da22a5f: no inaccuracies. The parenthetical names exactly the eight unvalidated scalars, and each raises at apply time. One nit predates this PR and is left alone: the sentence calling this "the one place" validation is deferred overlooks script_orders, which also defers to apply time.
  • Not run, as not applicable: type design (no Locale, Policy or PolicyPatch field or method changed, only a comment), test coverage (no test changed), design docs (nothing under docs/ or any AGENTS.md changed).

🤖 Generated with Claude Code

…comments

A quality-only pass over the 3.0 cycle's changes to _parser, _policy
and the locale packs; no behavior, message, repr or pickle changes.

- cs, sk, es, es_ar: DEVIATES is deviates_by_two_family_names itself,
  not a wrapper whose docstring restated (and in cs and sk had let go
  stale) the shared helper's
- parser_for: the per-pack tag check rewraps through _rewrapped, as
  the patch loop does (same text); the "one choice" comment moves to
  the loop it describes; rules/D2's second quote moves from above the
  warning re-emission to the apply_patch wrap it covers
- _no_strict_language: `if locales:` for a loop that ran at most once
- PolicyPatch: the comment naming every scalar field by name states
  the rule instead, so a new field cannot make it stale

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@derek73 derek73 added this to the 3.0 milestone Nov 5, 2026
@derek73 derek73 self-assigned this Nov 5, 2026
… is not a bool

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant