diff --git a/AGENTS.md b/AGENTS.md index 0cc15e3e..8060e880 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -298,7 +298,7 @@ Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_ **Titles permanently shadow first names — be conservative** — any word in `TITLES` is always consumed as a title and can never be parsed as a first name. `"Dean"` is the canonical example: it's a common academic title *and* a common given name, so it is intentionally absent from the default titles (see `docs/customize.rst` — users who need it add it via opt-in `Constants`). Before adding a word to `TITLES`, ask: "Could this plausibly be someone's given name in any culture?" If yes, don't add it globally; it belongs in caller-supplied `Constants` instead. This same caution applies to international honorifics — `Prince`, `Sheikh`, `Frau` are all first names in some contexts. It also applies to any prefix sub-set gated on "never a first name": obscure-looking foreign particles are surprisingly often real given names — `Von` (Von Miller), `Vander` (Brazilian, also the Arcane character). When unsure, exclude — a missing member just means that name isn't auto-handled, whereas a wrong member misparses a real person. -**The period-abbreviation title inference runs at the head of the GIVEN-NAME part, not the head of the name** — an unrecognized multi-letter word ending in a single trailing period (`_assign._PERIOD_ABBREV`, a hand copy of the `period_abbreviation` regex, `{2,}` letters) is treated as a title in the leading title run, e.g. `"Insp. Jane Morse"` → `title='Insp.'`. "Leading" is per SEGMENT: `_peel_leading_titles` is called for NO_COMMA segment 0, SUFFIX_COMMA segment 0, and FAMILY_COMMA **segment 1**, so `"Morse, Det. Insp. Jane"` → `title='Det. Insp.'` and a lone `"Smith, Xyz."` → `title='Xyz.'` — long-standing, verified against 1.4.0, and the mechanism behind #296 (`"Smith, Jr."` → title, which the shape rule claims even once `jr` leaves `TITLES`). The docs said "leading word" until 2026-08-01 and were wrong for every comma path. It does not mutate `C.titles`, so the periodless form (`"Insp"`) is unaffected elsewhere. The `{2,}` length requirement — not a separate initials check — is what excludes single-letter initials like `"J."`; the same word after the given name is left as a middle name. **The inference OUTRANKS vocabulary where it runs**: `"Esq. Smith"` → `title='Esq.'` even though `esq` is suffix-only vocabulary, because the shape rule fires before anything consults the suffix sets. **And it runs in one direction only**: a trailing abbreviation has no structural counterpart and is matched against the suffix vocabulary alone, so a trailing TITLE word is not a title (`"John Smith Prof."` → `family='Prof.'`, and the comma path disagrees — `"Smith, Prof."` → `title='Prof.'`). Meanwhile `period_joined_vocab` resolves INTERIOR-period tokens (`Lt.Gov.`, `Msc.Ed.`) to title-or-suffix by vocabulary, and `_extract._suffix_shaped` treats any period-final delimited content as not-a-nickname. Four trailing-period behaviors, four different resolutions; unifying them is open design work, not settled. (#109; see `docs/usage.rst` "Titles you didn't configure") +**The period-abbreviation title inference runs at the head of the GIVEN-NAME part, not the head of the name** — an unrecognized multi-letter word ending in a single trailing period (`_pieces._PERIOD_ABBREV`, a hand copy of the `period_abbreviation` regex, `{2,}` letters — it was assign's until #424 and group's until #439) is treated as a title in the leading title run, e.g. `"Insp. Jane Morse"` → `title='Insp.'`. "Leading" is per SEGMENT: `_peel_leading_titles` is called for NO_COMMA segment 0, SUFFIX_COMMA segment 0, and FAMILY_COMMA **segment 1**, so `"Morse, Det. Insp. Jane"` → `title='Det. Insp.'` and a lone `"Smith, Xyz."` → `title='Xyz.'` — long-standing, verified against 1.4.0, and the mechanism behind #296 (`"Smith, Jr."` → title, which the shape rule claims even once `jr` leaves `TITLES`). The docs said "leading word" until 2026-08-01 and were wrong for every comma path. It does not mutate `C.titles`, so the periodless form (`"Insp"`) is unaffected elsewhere. The `{2,}` length requirement — not a separate initials check — is what excludes single-letter initials like `"J."`; the same word after the given name is left as a middle name. **The inference OUTRANKS vocabulary where it runs**: `"Esq. Smith"` → `title='Esq.'` even though `esq` is suffix-only vocabulary, because the shape rule fires before anything consults the suffix sets. **And it runs in one direction only**: a trailing abbreviation has no structural counterpart and is matched against the suffix vocabulary alone, so a trailing TITLE word is not a title (`"John Smith Prof."` → `family='Prof.'`, and the comma path disagrees — `"Smith, Prof."` → `title='Prof.'`). Meanwhile `period_joined_vocab` resolves INTERIOR-period tokens (`Lt.Gov.`, `Msc.Ed.`) to title-or-suffix by vocabulary, and `_extract._suffix_shaped` treats any period-final delimited content as not-a-nickname. Four trailing-period behaviors, four different resolutions; unifying them is open design work, not settled. (#109; see `docs/usage.rst` "Titles you didn't configure") **Cyrillic suffix regexes need `re.I` even when the pattern is suffix-only** — a Latin title-cased word (`Ivanovich`) keeps its suffix lowercase, so `re.I` seemed skippable; but an irregular Cyrillic suffix can be nearly the whole word (`ильич`), so title-casing capitalizes into the suffix itself (`Ильич`). `east_slavic_patronymic_cyrillic` shipped without `re.I` on the Latin reasoning and silently failed on capitalized irregular forms — don't assume Latin's title-case safety transfers to Cyrillic. (#185) diff --git a/docs/design/mechanisms.md b/docs/design/mechanisms.md index cbaffa39..8fc606ab 100644 --- a/docs/design/mechanisms.md +++ b/docs/design/mechanisms.md @@ -53,9 +53,9 @@ Problem shape. A fact known during tokenization matters to a much later stage. C Problem shape. "Which stage does X?" — asked before attributing behavior in prose, comments, or fixes. Contract statement. Each stage's docstring header declares what it consumes, produces and reads, and ParseState's docstring holds the cross-stage map, pinned by tests/v2/pipeline/test_state.py. How it works. A claim about which stage or layer does something is CHECKABLE — `parse(s).tokens` prints every token's role and tags — so check it before writing it; one plausible attribution sentence once shipped six times wrong (AGENTS.md's stage-attribution note). Lives in. nameparser/_pipeline/_state.py and every stage header. Reach for it when. Writing any sentence of the form "X happens before Y sees it." -## ONE-PREDICATE-PER-QUESTION — the stage that does not decide calls the one that does +## ONE-PREDICATE-PER-QUESTION — one predicate answers it, and every other site calls that -Problem shape. Two stages need the same answer about the same input, and the one that does not own the decision is about to test for it. Contract statement. Where two sites ask the same question, exactly one predicate answers it, and the site that does not own the decision calls the deciding stage's own predicate — never a condition written to match it. How it works. A hand-written mirror agrees with its original only until one of them moves, and the drift is invisible in both directions: each site keeps passing its own tests while they disagree about an input neither covers. Five instances, every one found as a defect before it was found as a pattern — #319 lifted the wholly-suffix predicate into the vocabulary layer "so the comma decision and the honorific peel's segment test cannot drift apart"; #401/#421 lifted the trailing-numeral fork out of assign so the bound-given reserve stopped carrying a copy, its hand-written mirror having been falsified in review more than once — the lesson recorded there being that what must be mirrored is assign's WALK, not merely its condition; #425 replaced that reserve's hand re-derivation of the trailing peel with one function over the view the join would leave; #424 moved assign's leading-title test down because group's own `title()` does not see H2's unlisted abbreviations, so `Xyz. van Johnson` chained where `Dr. van Johnson` did not; #429 moved the no-name-segment test down because group asked by segment INDEX where assign asks by CONTENT. The destination follows the LAYER, not the topic: a predicate over token text goes to `_vocab`, one over pieces and tags goes to `_group` — not because grouping owns it, but because `_assign` imports `_group` and cannot be imported back. That import direction is this mechanism's limit, and it forecloses the alternative: where the reader comes AFTER the decider, record the answer on the state instead — `ParseState.order` is that shape, "Recorded rather than recomputed downstream, because the two can differ" — which is unavailable whenever the EARLIER stage is the one asking. The cost is a second evaluation of the same predicate, measured for #429 at 1.2–2.2% of a family-comma parse and 0% of every other; recording that number was the right answer there over plumbing a state field the two sites would not otherwise share. Lives in. nameparser/_pipeline/_vocab.py (is_wholly_suffix, is_trailing_numeral_suffix) and nameparser/_pipeline/_group.py (_is_suffix_piece, _is_leading_title, _leading_titles, _peel_walk, _peel_trailing, _segment_holds_no_name), each called by a stage that does not define it. Reach for it when. You are about to write a condition that mirrors, matches or "does what X does" — or you find a comment saying one does. Grep for the other site's predicate and call it instead. +Problem shape. Two stages need the same answer about the same input, and the one that does not own the decision is about to test for it. Contract statement. Where two sites ask the same question, exactly one predicate answers it and every other site calls that one — never a condition written to match it. The predicate belongs to the QUESTION, not to whichever stage decides: it may sit in a leaf both stages import, and for the leading-title test it must, since the deciding stage is assign and group cannot import assign. How it works. A hand-written mirror agrees with its original only until one of them moves, and the drift is invisible in both directions: each site keeps passing its own tests while they disagree about an input neither covers. Five instances, every one found as a defect before it was found as a pattern — #319 lifted the wholly-suffix predicate into the vocabulary layer "so the comma decision and the honorific peel's segment test cannot drift apart"; #401/#421 lifted the trailing-numeral fork out of assign so the bound-given reserve stopped carrying a copy, its hand-written mirror having been falsified in review more than once — the lesson recorded there being that what must be mirrored is assign's WALK, not merely its condition; #425 replaced that reserve's hand re-derivation of the trailing peel with one function over the view the join would leave; #424 moved assign's leading-title test down because group's own `title()` does not see H2's unlisted abbreviations, so `Xyz. van Johnson` chained where `Dr. van Johnson` did not; #429 moved the no-name-segment test down because group asked by segment INDEX where assign asks by CONTENT. The destination follows the LAYER, not the topic: a predicate over token text goes to `_vocab`, one over pieces and tags to `_pieces`. Both are leaves the stages sit on. The piece layer got its own module only in #439 — until then those predicates collected in `_group`, not because grouping owned them but because `_assign` imports `_group` and cannot be imported back, so group was the one place both stages could reach; five had accumulated across four PRs before the module existed. Stage order is this mechanism's limit, and it forecloses the alternative: where the reader comes AFTER the decider, record the answer on the state instead — `ParseState.order` is that shape, "Recorded rather than recomputed downstream, because the two can differ" — which is unavailable whenever the EARLIER stage is the one asking. (The concrete assign→group import that forced the `_group` collection is gone since #439; what remains is the ordering it was a symptom of, and tests/v2/test_layering.py is where the leaf's contract is now written down.) The cost is a second evaluation of the same predicate, measured for #429 at 1.2–2.2% of a family-comma parse and 0% of every other; recording that number was the right answer there over plumbing a state field the two sites would not otherwise share. Lives in. nameparser/_pipeline/_vocab.py over text (is_wholly_suffix, and is_trailing_numeral_suffix — the #401/#421 instance, whose only caller since #439 is the shared peel rather than a stage) and nameparser/_pipeline/_pieces.py over pieces: _is_suffix_piece, _is_leading_title, _leading_titles, _peel_walk, _peel_trailing and _segment_holds_no_name are called by both stages, _is_title_piece and _trailing_start by group alone — `_trailing_start` being the one to know, since it answers where the trailing run begins and is what P2's chain and M2's walk stop at. tests/v2/test_layering.py holds each module's contract, and a piece predicate growing a dependency on a STAGE shows up there as a widened entry. Reach for it when. You are about to write a condition that mirrors, matches or "does what X does" — or you find a comment saying one does. Grep for the other site's predicate and call it instead. ## CLAUSE-CONTENT-OVERRULES-DELIMITER — content wins diff --git a/docs/design/rules.md b/docs/design/rules.md index a1ab7bb9..fa7a6480 100644 --- a/docs/design/rules.md +++ b/docs/design/rules.md @@ -76,7 +76,7 @@ H2. Rationale: before a name, an abbreviation is almost always a not open: the vocabulary decides, and "Esq." is the postnominal it is. "Smith, Esq." → suffix="Esq." - history: decisions.md#H2 · interacts: C1, P4 · implemented: nameparser/_pipeline/_assign.py, nameparser/_pipeline/_group.py + history: decisions.md#H2 · interacts: C1, P4 · implemented: nameparser/_pipeline/_assign.py, nameparser/_pipeline/_pieces.py H3. Rationale: compound titles are written as a run of title words, connectives included; a title word standing inside the name is @@ -91,7 +91,7 @@ H3. Rationale: compound titles are written as a run of title words, Accepted: before a family comma the pre-comma text is wholly the family name (C1), title words included. "Dr. Smith, John" → family="Dr. Smith" - interacts: C1 · implemented: nameparser/_pipeline/_group.py + interacts: C1 · implemented: nameparser/_pipeline/_pieces.py ## Particles & surname prefixes (P) @@ -462,7 +462,7 @@ S2. Rationale: generational suffixes and credentials are recognized "Jack Wei Ma" → suffix="Ma" "Jack Wei Ma" → ambiguities=("suffix-or-name",) "Smith Jr." → family="" - implemented: nameparser/_pipeline/_classify.py, nameparser/_pipeline/_group.py, nameparser/_pipeline/_vocab.py + implemented: nameparser/_pipeline/_classify.py, nameparser/_pipeline/_group.py, nameparser/_pipeline/_pieces.py, nameparser/_pipeline/_vocab.py S3. Rationale: credentials are often written run together with periods; the chunks between the periods are what carry the diff --git a/nameparser/_pipeline/_assign.py b/nameparser/_pipeline/_assign.py index 3ab1d89c..6696597d 100644 --- a/nameparser/_pipeline/_assign.py +++ b/nameparser/_pipeline/_assign.py @@ -41,7 +41,7 @@ from nameparser._pipeline._vocab import ( effective_script, is_suffix_lenient, resolve_script_set, ) -from nameparser._pipeline._group import ( +from nameparser._pipeline._pieces import ( _is_suffix_piece, _leading_titles, _peel_trailing, _peel_walk, _segment_holds_no_name, ) @@ -60,7 +60,7 @@ def _set_roles(tokens: list[WorkToken], piece: tuple[int, ...], # rules.md#H2: "an abbreviation opening the part of the name that # carries the given name — the whole name, or the part after a # family comma — reads as a title even when unlisted" -- the count is -# group's _leading_titles since #424 (its test, _is_leading_title, is +# _pieces._leading_titles since #424 (its test, _is_leading_title, is # the leading-particle scan's too); the roles are set here. def _peel_leading_titles(pieces: tuple[tuple[int, ...], ...], ptags: tuple[frozenset[str], ...], @@ -188,7 +188,7 @@ def _assign_main(seg_idx: int, state: ParseState, _set_roles(tokens, pieces[rest[0]], Role.FAMILY) return None # peel the trailing suffix run: k = first index in rest from which - # every piece is a suffix. The walk is group's _peel_trailing since + # every piece is a suffix. The walk is _pieces._peel_trailing since # #425 -- one walk, shared with the bound-given reserve, and # documented there. Every bare ambiguous acronym it had to resolve # is one coin-flip each, in either direction, so the report diff --git a/nameparser/_pipeline/_group.py b/nameparser/_pipeline/_group.py index e7737ba2..bfe0050d 100644 --- a/nameparser/_pipeline/_group.py +++ b/nameparser/_pipeline/_group.py @@ -13,40 +13,44 @@ registration becomes piece_tags entries -- per-parse state that dissolves with the state (v1 kept per-parse sets for the same reason). -Implements rules H3, P2, P3, P4 and M2, and houses H2's test of docs/design/rules.md and the +Implements rules P2, P3, P4 and M2, and the group half of M1 (#329: the marker dropped inside EXTRACTED maiden content, which M2's pieces walk cannot reach because extract's content never enters pieces); each is cited at its code below. Also implements rule P5 (cited below at the bound-given join) and ports -the "Ph. D."-split merge (v1 fix_phd; decisions.md#phd-merge). Houses -the S2 trailing peel (_peel_walk, _peel_trailing, _trailing_start), a -piece-level walk that assign applies and that P5's reserve, P2's -chain and M2's walk read (#425, #424), and assign's leading-title -test (_is_leading_title, _leading_titles), which the chain's scan -shares since #424, and its no-name-segment test -(_segment_holds_no_name), which the one-entry join reads since #429. -These are here by import direction, not by topic -(mechanisms.md#ONE-PREDICATE-PER-QUESTION): assign imports group and -cannot be imported back, so a piece-level predicate both stages ask -lives here. A text-level one goes to _vocab instead. +the "Ph. D."-split merge (v1 fix_phd; decisions.md#phd-merge). + +The piece-level predicates moved to _pieces in #439 -- the S2 +trailing peel, the leading-title and title-piece tests, the +suffix-piece test, the no-name-segment test. Most are shared with +assign; _is_title_piece and _trailing_start are group's alone and +travelled because the shared ones call them. They had collected here +by import direction rather than by topic (assign imported group and +could not be imported back), which is the accumulation +mechanisms.md#ONE-PREDICATE-PER-QUESTION describes; group imports them +back like any other caller, and still does the work H3 and S2 describe +with them. What remains defined here is group's own: _is_prefix_piece, +_is_conj_piece, _is_rootname and _is_maiden_marker_piece. """ from __future__ import annotations import bisect import dataclasses -import re from collections.abc import Sequence, Set from enum import IntEnum -from typing import NamedTuple from nameparser._lexicon import _title_key +from nameparser._pipeline._pieces import ( + _is_leading_title, _is_suffix_piece, _is_title_piece, + _leading_titles, _peel_trailing, _peel_walk, _segment_holds_no_name, + _trailing_start, +) from nameparser._pipeline._state import ( ParseState, PendingAmbiguity, Structure, WorkToken, ) from nameparser._pipeline._vocab import D as _D from nameparser._pipeline._vocab import PH as _PH from nameparser._pipeline._vocab import delimiter_cores -from nameparser._pipeline._vocab import is_trailing_numeral_suffix from nameparser._types import AmbiguityKind, Role # the credential-pair regexes live in _vocab, whose own @@ -75,56 +79,10 @@ class BoundJoin(IntEnum): STRICT = 2 # main segments (reserve_last=True: keep a family piece) -# rules.md#H3: "successive title words at the start of the part -# carrying the given name chain into one title; a title word -# elsewhere in the name does not" -def _is_title_piece(piece: Sequence[int], ptags: Set[str], - tokens: Sequence[WorkToken]) -> bool: - if "title" in ptags: - return True - return len(piece) == 1 and "vocab:title" in tokens[piece[0]].tags - - -# Ported verbatim from v1 (nameparser/config/regexes.py -# "period_abbreviation") -- layering forbids the config import; keep -# in sync by hand (tests/v2/test_regex_sync.py). Here rather than in -# assign since #424: the leading-title test is assign's, and group's -# leading-particle scan and trailing-run walk must start where assign -# starts. -_PERIOD_ABBREV = re.compile(r'^[^\W\d_]{2,}\.$') - - -# rules.md#H2: "an abbreviation opening the part of the name that -# carries the given name — the whole name, or the part after a -# family comma — reads as a title even when unlisted" -# (history: decisions.md#H2) -def _is_leading_title(piece: Sequence[int], ptags: Set[str], - tokens: Sequence[WorkToken]) -> bool: - if _is_title_piece(piece, ptags, tokens): - return True - return (len(piece) == 1 - and bool(_PERIOD_ABBREV.match(tokens[piece[0]].text))) - - -def _leading_titles(pieces: Sequence[Sequence[int]], - ptags: Sequence[Set[str]], - tokens: Sequence[WorkToken]) -> int: - """How many leading pieces assign peels as titles: the first - non-title index. A title needs a following piece, unless the whole - segment is one title (v1 parity). One definition, read by assign - (which sets the roles) and by the chain's trailing-run walk; the - leading-particle scan shares the predicate, _is_leading_title, - but stops at a title-and-particle word (P4, #367, #424).""" - n = 0 - while n < len(pieces): - if ((n + 1 < len(pieces) or len(pieces) == 1) - and _is_leading_title(pieces[n], ptags[n], tokens)): - n += 1 - continue - break - return n - - +# rules.md#S2: "a trailing word of the suffix vocabulary reads as a +# suffix" -- group does not decide that; it stops before whatever +# _trailing_start says the run is, so the chain and the maiden walk +# end where assign's peel begins (#424). # rules.md#P2: "a particle joins the words after it into one name # part, the join running until the next particle starts a group of # its own, a trailing suffix begins" -- and on to the maiden marker @@ -139,169 +97,6 @@ def _is_prefix_piece(piece: Sequence[int], ptags: Set[str], return len(piece) == 1 and "particle" in tokens[piece[0]].tags -def _is_suffix_piece(piece: Sequence[int], ptags: Set[str], - tokens: Sequence[WorkToken]) -> bool: - if "suffix" in ptags: - return True - if len(piece) != 1: - return False - tags = tokens[piece[0]].tags - return "vocab:suffix" in tags and "initial" not in tags - - -def _segment_holds_no_name(pieces: Sequence[Sequence[int]], - ptags: Sequence[Set[str]], - tokens: Sequence[WorkToken]) -> bool: - """The segment is titles and suffixes only ('John Smith, Dr.', - 'John Smith, Mr. Jr.') -- nothing in it is a name word. - - The FAMILY_COMMA rule "segment 0 is wholly the family name" rests on - the writer having said where the family name ends. A comma followed - by no name word said no such thing -- 'John Smith, Dr.' is 'Dr. John - Smith' with the honorific moved, and 'John Smith, Mr. Jr.' the same - with the postnominal along -- so the pre-comma name keeps its - positional read instead of being merged. Uses the same - _is_leading_title predicate the peel does, period-abbreviation - inference included, so the two cannot disagree about what a title - is; a suffix piece counts as what it is, so a mixed run like - 'Smith, Dr. Jr.' is a title and a postnominal, each read where it - stands, and never a title run 'Dr. Jr.'. An empty segment - ('Doe,, Jr.') holds no title to read by. - - TWO callers, asking it for different reasons, and the difference - matters. assign uses it to decide whether the comma fixed the family - name (above). group's one-entry join (#429) uses it to decide - whether the segment is a credential run at all. - - True does NOT mean "every piece is a suffix" -- the title tolerance - is the whole point, and a true segment can still hold pieces assign - routes to TITLE, so a caller rendering the segment as one unit must - ask _is_suffix_piece per piece as well. What assuming otherwise cost - is recorded at the one-entry join in group(), the caller that made - the assumption. - """ - if not pieces: - return False - return all(_is_suffix_piece(pieces[k], ptags[k], tokens) - or _is_leading_title(pieces[k], ptags[k], tokens) - for k in range(len(pieces))) - - -class Peel(NamedTuple): - """What assign's trailing peel made of a walk. `names` is a count - of positions in the caller's `rest`: rest[:names] are the name - pieces and rest[names:] the suffixes. The other two are pieces -- - token-index tuples, as PendingAmbiguity wants them -- and each is - one token long: `numeral` is the piece the roman-numeral fork - took (None when it did not fire; always the walk's last piece), - `picks` the bare ambiguous acronyms the peel had to resolve, in - peel order, either way (the last may sit at rest[names - 1]).""" - - names: int - numeral: tuple[int, ...] | None - picks: tuple[tuple[int, ...], ...] - - -# rules.md#S2: "a trailing word of the suffix vocabulary reads as a -# suffix — generational forms and credential acronyms alike, and an -# ambiguous acronym written with its periods, one after each letter, -# counts unambiguously; a single trailing period is the abbreviation -# shape any word can wear and does not. A BARE ambiguous acronym is -# consumed only when the name has words to spare" -# (v1's are_suffixes tail rule, with the roman-numeral special) -def _peel_walk(start: int, ptags: Sequence[Set[str]], - skip: Set[int] = frozenset()) -> list[int]: - """The indices _peel_trailing walks: `start` to the segment's end, - minus the group-flagged credential pieces (the Ph. D. merge), - which assign reads as suffixes at any position, and minus `skip` - -- a tail segment's delimiter cores, which are structure rather - than words (the maiden walk's case, #424). Built here and nowhere - else, so the walk's input cannot drift between assign and the - three group sites that read it: the numeral fork is a last-piece - test that reads the piece before as rest[k - 2], which holds only - over this list.""" - return [j for j in range(start, len(ptags)) - if j not in skip and "suffix" not in ptags[j]] - - -def _trailing_start(start: int, pieces: Sequence[Sequence[int]], - ptags: Sequence[Set[str]], tokens: Sequence[WorkToken], - skip: Set[int] = frozenset(), - numeral_only: bool = False) -> int: - """Where assign's trailing suffix run begins, read over the pieces - as they stand from `start`: the index of the first piece the S2 - peel takes, or len(pieces) when it takes none (#424). What P2's - chain and M2's walk stop before -- each had asked "is this a - suffix?" with the suffix-piece test, which vetoes a bare 'V' as - an initial (the #401 question), and so took a trailing numeral, - or a bare acronym with words to spare, into the family or the - maiden name. - - `numeral_only` is the maiden walk's reading: the bare-acronym - fork counts pieces, and the walk removes the very pieces it - counted, so an acronym peeled over the pieces as they stand may - be the family of what is left ('John née Jones Smith Ma' read - maiden 'Jones Smith', family 'Ma'). The numeral fork reads one - piece, the one before the numeral, and _maiden_take re-asks it - with the piece the take leaves there; the acronym is left to - assign.""" - rest = _peel_walk(start, ptags, skip) - peeled = _peel_trailing(rest, pieces, ptags, tokens) - if numeral_only: - return rest[-1] if peeled.numeral is not None else len(pieces) - return rest[peeled.names] if peeled.names < len(rest) else len(pieces) - - -def _peel_trailing(rest: Sequence[int], pieces: Sequence[Sequence[int]], - ptags: Sequence[Set[str]], - tokens: Sequence[WorkToken]) -> Peel: - """The S2 trailing peel over `rest`, a _peel_walk list. Housed - here rather than in assign because assign imports group's piece - predicates, and group's bound-given reserve asks the same question - of the view the join would leave (#425): one walk, so the reserve - and the assignment cannot drift. Pure -- the ambiguities are - returned for assign to report, in the order it always reported - them.""" - picks: list[tuple[int, ...]] = [] - numeral: tuple[int, ...] | None = None - k = len(rest) - while k > 0: - piece = pieces[rest[k - 1]] - if _is_suffix_piece(piece, ptags[rest[k - 1]], tokens): - k -= 1 - continue - # a final single letter that is a roman numeral, after a piece - # that is not initial-shaped; the predicate's docstring carries - # the is_initial_shaped reasoning (#320) - if (k == len(rest) and k >= 2 and len(piece) == 1 - and is_trailing_numeral_suffix( - tokens[piece[0]].text, - tokens[pieces[rest[k - 2]][0]].text)): - numeral = tuple(piece) - k -= 1 - continue - # A bare ambiguous acronym ("MA", not "M.A.") is a credential - # only when peeling it still leaves a given AND a family name. - # With two pieces, "one of them is a credential" is the less - # likely reading, so it stays the family name -- "Jack MA" is a - # person, "John Smith MA" is a person with a degree. This is - # v1's reserve_last narrowed to the ambiguous set: 2.0 - # deliberately peels UNambiguous suffixes even when nothing is - # left ("Smith PhD" -> suffix, a classified fix), because there - # the vocabulary is not in doubt. - bare_ambiguous = (len(piece) == 1 - and "vocab:suffix-ambiguous" in tokens[piece[0]].tags) - # k < 2 means it is the only piece left, which is not the fork - # this reports. - if bare_ambiguous and k >= 2: - picks.append(tuple(piece)) - if k >= 3: # peeling still leaves given + family - k -= 1 - continue - break - return Peel(k, numeral, tuple(picks)) - - # rules.md#M2: "a recognized maiden marker standing after at least # one name word takes the words after it" -- up to any suffix word, # or the trailing numeral assign reads as the suffix, as the maiden diff --git a/nameparser/_pipeline/_pieces.py b/nameparser/_pipeline/_pieces.py new file mode 100644 index 00000000..f227d026 --- /dev/null +++ b/nameparser/_pipeline/_pieces.py @@ -0,0 +1,253 @@ +"""Shared piece-level predicates for pipeline stages. + +How a PIECE reads -- its tokens, plus the tags classify wrote on them +and the tags group derived for the piece -- where _vocab answers how a +WORD reads from text alone. Both are consulted by more than one +stage; the split is by what the question takes, not by +which stage happens to ask (mechanisms.md#ONE-PREDICATE-PER-QUESTION). +_vocab points here from its own side: "Text-level tests used by more +than one stage; piece-level ones live in _pieces, the sibling layer +over tokens-plus-tags." + +Before this module those predicates lived in _group, not because +grouping owned them but because assign imported group and could not be +imported back, so group was the only place both stages could reach. +They arrived there that way across three PRs -- #424 brought +_is_leading_title, _leading_titles and _trailing_start, #425 the peel +(_peel_walk, _peel_trailing), #429 _segment_holds_no_name. +_is_title_piece and _is_suffix_piece are older than any of that: they +were group's from its first commit, and travel because the others +call them. + +The import that forced all of it is the one #439 removed: assign no +longer names _group at all. What still holds is the rule that replaced +it, and tests/v2/test_layering.py is where it is written down -- a +piece predicate may not depend on a stage, in either direction. + +The S2 trailing peel travels as the unit decisions.md describes -- +_peel_walk, _peel_trailing and _trailing_start together -- though only +the first two cross a stage boundary. + +Layering: imports _state and _vocab only; _group and _assign import +it, and neither of the two it imports imports it back. +""" +from __future__ import annotations + +import re +from collections.abc import Sequence, Set +from typing import NamedTuple + +from nameparser._pipeline._state import WorkToken +from nameparser._pipeline._vocab import is_trailing_numeral_suffix + + +# rules.md#H3: "successive title words at the start of the part +# carrying the given name chain into one title; a title word +# elsewhere in the name does not" +def _is_title_piece(piece: Sequence[int], ptags: Set[str], + tokens: Sequence[WorkToken]) -> bool: + if "title" in ptags: + return True + return len(piece) == 1 and "vocab:title" in tokens[piece[0]].tags + + +# Ported verbatim from v1 (nameparser/config/regexes.py +# "period_abbreviation") -- layering forbids the config import; keep +# in sync by hand (tests/v2/test_regex_sync.py). Out of assign since +# #424 and in the piece layer since #439: the test is assign's, and group's +# leading-particle scan and trailing-run walk must start where assign +# starts. +_PERIOD_ABBREV = re.compile(r'^[^\W\d_]{2,}\.$') + + +# rules.md#H2: "an abbreviation opening the part of the name that +# carries the given name — the whole name, or the part after a +# family comma — reads as a title even when unlisted" +# (history: decisions.md#H2) +def _is_leading_title(piece: Sequence[int], ptags: Set[str], + tokens: Sequence[WorkToken]) -> bool: + if _is_title_piece(piece, ptags, tokens): + return True + return (len(piece) == 1 + and bool(_PERIOD_ABBREV.match(tokens[piece[0]].text))) + + +def _leading_titles(pieces: Sequence[Sequence[int]], + ptags: Sequence[Set[str]], + tokens: Sequence[WorkToken]) -> int: + """How many leading pieces assign peels as titles: the first + non-title index. A title needs a following piece, unless the whole + segment is one title (v1 parity). One definition, read by assign + (which sets the roles) and by the chain's trailing-run walk; the + leading-particle scan shares the predicate, _is_leading_title, + but stops at a title-and-particle word (P4, #367, #424).""" + n = 0 + while n < len(pieces): + if ((n + 1 < len(pieces) or len(pieces) == 1) + and _is_leading_title(pieces[n], ptags[n], tokens)): + n += 1 + continue + break + return n + + +def _is_suffix_piece(piece: Sequence[int], ptags: Set[str], + tokens: Sequence[WorkToken]) -> bool: + if "suffix" in ptags: + return True + if len(piece) != 1: + return False + tags = tokens[piece[0]].tags + return "vocab:suffix" in tags and "initial" not in tags + + +def _segment_holds_no_name(pieces: Sequence[Sequence[int]], + ptags: Sequence[Set[str]], + tokens: Sequence[WorkToken]) -> bool: + """The segment is titles and suffixes only ('John Smith, Dr.', + 'John Smith, Mr. Jr.') -- nothing in it is a name word. + + The FAMILY_COMMA rule "segment 0 is wholly the family name" rests on + the writer having said where the family name ends. A comma followed + by no name word said no such thing -- 'John Smith, Dr.' is 'Dr. John + Smith' with the honorific moved, and 'John Smith, Mr. Jr.' the same + with the postnominal along -- so the pre-comma name keeps its + positional read instead of being merged. Uses the same + _is_leading_title predicate the peel does, period-abbreviation + inference included, so the two cannot disagree about what a title + is; a suffix piece counts as what it is, so a mixed run like + 'Smith, Dr. Jr.' is a title and a postnominal, each read where it + stands, and never a title run 'Dr. Jr.'. An empty segment + ('Doe,, Jr.') holds no title to read by. + + TWO callers, asking it for different reasons, and the difference + matters. assign uses it to decide whether the comma fixed the family + name (above). group's one-entry join (#429) uses it to decide + whether the segment is a credential run at all. + + True does NOT mean "every piece is a suffix" -- the title tolerance + is the whole point, and a true segment can still hold pieces assign + routes to TITLE, so a caller rendering the segment as one unit must + ask _is_suffix_piece per piece as well. What assuming otherwise cost + is recorded at the one-entry join in group(), the caller that made + the assumption. + """ + if not pieces: + return False + return all(_is_suffix_piece(pieces[k], ptags[k], tokens) + or _is_leading_title(pieces[k], ptags[k], tokens) + for k in range(len(pieces))) + + +class Peel(NamedTuple): + """What assign's trailing peel made of a walk. `names` is a count + of positions in the caller's `rest`: rest[:names] are the name + pieces and rest[names:] the suffixes. The other two are pieces -- + token-index tuples, as PendingAmbiguity wants them -- and each is + one token long: `numeral` is the piece the roman-numeral fork + took (None when it did not fire; always the walk's last piece), + `picks` the bare ambiguous acronyms the peel had to resolve, in + peel order, either way (the last may sit at rest[names - 1]).""" + + names: int + numeral: tuple[int, ...] | None + picks: tuple[tuple[int, ...], ...] + + +# rules.md#S2: "a trailing word of the suffix vocabulary reads as a +# suffix — generational forms and credential acronyms alike, and an +# ambiguous acronym written with its periods, one after each letter, +# counts unambiguously; a single trailing period is the abbreviation +# shape any word can wear and does not. A BARE ambiguous acronym is +# consumed only when the name has words to spare" +# (v1's are_suffixes tail rule, with the roman-numeral special) +def _peel_walk(start: int, ptags: Sequence[Set[str]], + skip: Set[int] = frozenset()) -> list[int]: + """The indices _peel_trailing walks: `start` to the segment's end, + minus the group-flagged credential pieces (the Ph. D. merge), + which assign reads as suffixes at any position, and minus `skip` + -- a tail segment's delimiter cores, which are structure rather + than words (the maiden walk's case, #424). Built here and nowhere + else, so the walk's input cannot drift between assign and the + group sites that read it: the numeral fork is a last-piece + test that reads the piece before as rest[k - 2], which holds only + over this list.""" + return [j for j in range(start, len(ptags)) + if j not in skip and "suffix" not in ptags[j]] + + +def _trailing_start(start: int, pieces: Sequence[Sequence[int]], + ptags: Sequence[Set[str]], tokens: Sequence[WorkToken], + skip: Set[int] = frozenset(), + numeral_only: bool = False) -> int: + """Where assign's trailing suffix run begins, read over the pieces + as they stand from `start`: the index of the first piece the S2 + peel takes, or len(pieces) when it takes none (#424). What P2's + chain and M2's walk stop before -- each had asked "is this a + suffix?" with the suffix-piece test, which vetoes a bare 'V' as + an initial (the #401 question), and so took a trailing numeral, + or a bare acronym with words to spare, into the family or the + maiden name. + + `numeral_only` is the maiden walk's reading: the bare-acronym + fork counts pieces, and the walk removes the very pieces it + counted, so an acronym peeled over the pieces as they stand may + be the family of what is left ('John née Jones Smith Ma' read + maiden 'Jones Smith', family 'Ma'). The numeral fork reads one + piece, the one before the numeral, and _maiden_take re-asks it + with the piece the take leaves there; the acronym is left to + assign.""" + rest = _peel_walk(start, ptags, skip) + peeled = _peel_trailing(rest, pieces, ptags, tokens) + if numeral_only: + return rest[-1] if peeled.numeral is not None else len(pieces) + return rest[peeled.names] if peeled.names < len(rest) else len(pieces) + + +def _peel_trailing(rest: Sequence[int], pieces: Sequence[Sequence[int]], + ptags: Sequence[Set[str]], + tokens: Sequence[WorkToken]) -> Peel: + """The S2 trailing peel over `rest`, a _peel_walk list. In the + piece layer rather than in assign because group's bound-given + reserve asks the same question of the view the join would leave + (#425): one walk, so the reserve and the assignment cannot drift. Pure -- the ambiguities are + returned for assign to report, in the order it always reported + them.""" + picks: list[tuple[int, ...]] = [] + numeral: tuple[int, ...] | None = None + k = len(rest) + while k > 0: + piece = pieces[rest[k - 1]] + if _is_suffix_piece(piece, ptags[rest[k - 1]], tokens): + k -= 1 + continue + # a final single letter that is a roman numeral, after a piece + # that is not initial-shaped; the predicate's docstring carries + # the is_initial_shaped reasoning (#320) + if (k == len(rest) and k >= 2 and len(piece) == 1 + and is_trailing_numeral_suffix( + tokens[piece[0]].text, + tokens[pieces[rest[k - 2]][0]].text)): + numeral = tuple(piece) + k -= 1 + continue + # A bare ambiguous acronym ("MA", not "M.A.") is a credential + # only when peeling it still leaves a given AND a family name. + # With two pieces, "one of them is a credential" is the less + # likely reading, so it stays the family name -- "Jack MA" is a + # person, "John Smith MA" is a person with a degree. This is + # v1's reserve_last narrowed to the ambiguous set: 2.0 + # deliberately peels UNambiguous suffixes even when nothing is + # left ("Smith PhD" -> suffix, a classified fix), because there + # the vocabulary is not in doubt. + bare_ambiguous = (len(piece) == 1 + and "vocab:suffix-ambiguous" in tokens[piece[0]].tags) + # k < 2 means it is the only piece left, which is not the fork + # this reports. + if bare_ambiguous and k >= 2: + picks.append(tuple(piece)) + if k >= 3: # peeling still leaves given + family + k -= 1 + continue + break + return Peel(k, numeral, tuple(picks)) diff --git a/nameparser/_pipeline/_vocab.py b/nameparser/_pipeline/_vocab.py index 98c1d7d2..f9e697d2 100644 --- a/nameparser/_pipeline/_vocab.py +++ b/nameparser/_pipeline/_vocab.py @@ -1,7 +1,7 @@ """Shared vocabulary predicates for pipeline stages. -Text-level tests used by more than one stage; token/piece-level -predicates live with their stage. All take normalized-or-raw text +Text-level tests used by more than one stage; piece-level ones live +in _pieces, the sibling layer over tokens-plus-tags. All take normalized-or-raw text explicitly -- no state. is_wholly_suffix departs from that shape twice, deliberately. It is diff --git a/tests/v2/test_layering.py b/tests/v2/test_layering.py index f3c35fbd..70210512 100644 --- a/tests/v2/test_layering.py +++ b/tests/v2/test_layering.py @@ -4,6 +4,7 @@ import pathlib import nameparser +import nameparser._pipeline PKG = pathlib.Path(nameparser.__file__).parent @@ -17,6 +18,7 @@ "_pipeline/_vocab.py", "_pipeline/_script_segment.py", "_pipeline/_segment.py", "_pipeline/_classify.py", "_pipeline/_group.py", + "_pipeline/_pieces.py", "_pipeline/_assign.py", "_pipeline/_post_rules.py", "_pipeline/_assemble.py", "_parser.py", "_facade.py", "_config_shim.py", "locales/__init__.py", "locales/ja.py", @@ -24,8 +26,8 @@ _PIPELINE_STAGE_ALLOWED = ( "nameparser._types", "nameparser._lexicon", "nameparser._policy", - # stages share _state plus in-package helpers (_vocab, _group's - # piece predicates); the prefix still forbids _render/_locale/_parser + # stages share _state plus in-package helpers (_vocab and + # _pieces); the prefix still forbids _render/_locale/_parser "nameparser._pipeline.", ) @@ -69,6 +71,18 @@ "_pipeline/_script_segment.py": _PIPELINE_STAGE_ALLOWED, "_pipeline/_segment.py": _PIPELINE_STAGE_ALLOWED, "_pipeline/_classify.py": _PIPELINE_STAGE_ALLOWED, + # Piece-level predicates shared by group and assign + # (mechanisms.md#ONE-PREDICATE-PER-QUESTION). Tighter than the + # stage allowance on purpose: it is a leaf both stages sit on, so + # it may read the token type and the vocabulary layer and NOTHING + # else -- not even _lexicon, which is the likeliest next reach (a + # title predicate wanting _title_key) and so the one this entry + # most needs to refuse. Carrying the stage allowance's other + # prefixes here would pre-authorise the very widening the entry + # exists to make visible. Widening it is the tell that a piece + # predicate has grown a dependency on a stage. + "_pipeline/_pieces.py": ("nameparser._pipeline._state", + "nameparser._pipeline._vocab"), "_pipeline/_group.py": _PIPELINE_STAGE_ALLOWED, "_pipeline/_assign.py": _PIPELINE_STAGE_ALLOWED, "_pipeline/_post_rules.py": _PIPELINE_STAGE_ALLOWED, @@ -167,6 +181,48 @@ def test_layering_contract() -> None: ) +def test_every_pipeline_module_is_keyed_in_allowed() -> None: + """ALLOWED is a hand-maintained list, and test_layering_contract + iterates ITS keys -- so a module absent from it is not checked + loosely, it is not checked AT ALL. Nothing else notices: the suite + stays green, and the new module may import whatever it likes. + + Scoped to _pipeline/ because that is what this contract governs. + Unkeyed by choice: the config/ DATA modules (config/__init__.py is + keyed, being code), util.py (no internal imports, and it dies with + the 1.x layer) and _version.py. nameparser/__init__.py is unkeyed + too and is none of those -- it is the export surface, held by + test_public_exports instead. + + Measured when this was added: all 13 _pipeline modules were keyed, + so the gap was latent rather than active. #439's _pieces.py would + have been the first to slip through, which is why closing it then + cost nothing to clean up. + + The directory comes from the imported subpackage rather than a path + literal, so a typo is an AttributeError; and the floor below is not + ceremony. Path.glob on a missing directory returns EMPTY rather than + raising, so a mistyped pattern would leave this asserting `set() - + keyed`, which passes for every possible ALLOWED -- a completeness + check measuring nothing, which is the shape of the bug it exists + to prevent. + """ + keyed = set(ALLOWED) + pipeline_dir = pathlib.Path(nameparser._pipeline.__file__).parent + shipped = {f"_pipeline/{p.name}" for p in pipeline_dir.glob("*.py")} + assert "_pipeline/_group.py" in shipped, ( + f"the glob matched no pipeline modules, so this check can no " + f"longer fail: {sorted(shipped)}") + missing = sorted(shipped - keyed) + assert not missing, ( + f"pipeline modules absent from ALLOWED, and therefore exempt " + f"from the layering contract entirely: {missing}. Add each with " + f"the narrowest prefix tuple that admits what it actually " + f"imports -- _PIPELINE_STAGE_ALLOWED for a stage, something " + f"tighter for a leaf both stages sit on (see _pipeline/_pieces.py)" + ) + + def test_lexicon_never_imports_config_package_root_or_parser() -> None: for imported in _nameparser_imports(PKG / "_lexicon.py"): assert imported != "nameparser.config" diff --git a/tests/v2/test_regex_sync.py b/tests/v2/test_regex_sync.py index cf8d8b01..61cd4a24 100644 --- a/tests/v2/test_regex_sync.py +++ b/tests/v2/test_regex_sync.py @@ -18,14 +18,17 @@ are pinned in test_ledger_guards.py, which shares nothing with this module but the idea (#352). """ +import pathlib import re import pytest from nameparser.config import regexes as _config from nameparser._pipeline import ( - _assign, _group, _post_rules, _tokenize, _vocab, + _assemble, _assign, _classify, _extract, _group, _pieces, + _post_rules, _script_segment, _segment, _state, _tokenize, _vocab, ) +import nameparser._pipeline from nameparser import _render @@ -46,8 +49,8 @@ def test_period_not_at_end_matches_config() -> None: def test_period_abbreviation_matches_config() -> None: source = _config.REGEXES["period_abbreviation"] - assert _group._PERIOD_ABBREV.pattern == source.pattern - assert _group._PERIOD_ABBREV.flags == source.flags + assert _pieces._PERIOD_ABBREV.pattern == source.pattern + assert _pieces._PERIOD_ABBREV.flags == source.flags def test_roman_numeral_matches_config() -> None: @@ -99,7 +102,7 @@ def test_initial_copies_agree_with_each_other_and_config() -> None: # completeness check below: adding a pattern without declaring its # source now fails here instead of being silently unpinned. _SOURCES: dict[tuple[str, str], str | None] = { - ("_group", "_PERIOD_ABBREV"): "period_abbreviation", + ("_pieces", "_PERIOD_ABBREV"): "period_abbreviation", ("_group", "_D"): None, ("_vocab", "_DOTTED"): None, ("_group", "_PH"): None, @@ -122,9 +125,43 @@ def test_initial_copies_agree_with_each_other_and_config() -> None: ("_render", "_COMMA_CHAR"): None, } -_MODULES = {"_assign": _assign, "_group": _group, - "_post_rules": _post_rules, - "_render": _render, "_tokenize": _tokenize, "_vocab": _vocab} +def test_every_pipeline_module_is_scanned_for_hand_copies() -> None: + """The twin of test_layering's completeness check, in the file with + the same shape of registry. + + test_every_hand_copied_pattern_is_declared iterates _MODULES, so a + module absent from that dict is not scanned loosely -- it is not + scanned at all, and an undeclared hand copy of a config regex in it + goes unpinned. Measured on a scratch copy: a new pipeline module + carrying a wrong copy of the mac pattern passed all 20 tests here. + + Scoped to _pipeline/ for the same reason the layering twin is: it + is where a new module is a design decision. The directory is + derived from the imported subpackage and the floor is asserted -- + Path.glob on a missing directory returns empty rather than raising, + which would make this pass for every possible _MODULES. + """ + pipeline_dir = pathlib.Path(nameparser._pipeline.__file__).parent + shipped = {p.stem for p in pipeline_dir.glob("*.py") + if p.stem != "__init__"} + assert "_group" in shipped, ( + f"the glob matched no pipeline modules, so this check can no " + f"longer fail: {sorted(shipped)}") + missing = sorted(shipped - set(_MODULES)) + assert not missing, ( + f"pipeline modules absent from _MODULES, and therefore never " + f"scanned for undeclared hand copies of a config regex: " + f"{missing}. Add each one; a module with no hand copies costs " + f"nothing to scan" + ) + + +_MODULES = {"_assemble": _assemble, "_assign": _assign, + "_classify": _classify, "_extract": _extract, + "_group": _group, "_pieces": _pieces, + "_post_rules": _post_rules, "_render": _render, + "_script_segment": _script_segment, "_segment": _segment, + "_state": _state, "_tokenize": _tokenize, "_vocab": _vocab} @pytest.mark.parametrize(