Skip to content

Commit 30eb338

Browse files
derek73claude
andcommitted
Give each SUFFIX_OR_NAME branch its own detail, and stop over-claiming
Two findings from the review. The kind has one name and two causes, and they were sharing one template. The acronym branch turns on whether periods are written; the numeral branch turns on the letter being a numeral at all. Sharing the first produced: 'V' written without periods is both a post-nominal and an ordinary name; read as a suffix rather than a name part "written without periods" is the MA/M.A. distinction, which does not exist for V, and the flattened "a name part" hid that the declined reading is specifically a middle initial. The kind's own docstring says detail names what was declined; it could not even tell you which branch fired. Each branch now writes its own. And the documented claim is narrowed, for the second time. The review found a THIRD silent site for the roman-numeral fork -- the SUFFIX_COMMA tail, decided in _segment by is_suffix_lenient and stamped in _assign's blanket tail loop, so it is neither of the two comma exceptions already documented. "John Smith, V" reports nothing. Rather than chase coverage a third time, the claim no longer depends on it. It said an empty tuple means "none of the forks listed here came up", which is only true if every listed kind is emitted everywhere its fork occurs -- a property I cannot verify and have now been wrong about twice. It now says reporting is deliberately partial, names the comma paths as the known quiet ones, and states the only thing that holds regardless: a non-empty tuple is a signal, an empty one is not a guarantee. Verified: corpus 486 names, 2 intentional diffs, 0 unexplained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent de264af commit 30eb338

4 files changed

Lines changed: 47 additions & 25 deletions

File tree

docs/concepts.rst

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,13 @@ before it arises, too: ``"Ma, Jack"`` fixes the family name, so the
163163
credential reading never comes up, while ``"John Smith MA"`` has to
164164
call it and says so.
165165

166-
So an empty ``ambiguities`` does not mean every word was unambiguous —
167-
it means none of the forks the parser *reports on* came up. The kinds
168-
it reports are listed in :class:`~nameparser.AmbiguityKind`, and they
169-
are the ones where both readings are genuinely common in real names.
170-
Coverage grows over releases; a name that reports nothing today may
171-
report something later, so treat a non-empty ``ambiguities`` as a
172-
signal worth acting on rather than an empty one as a guarantee.
166+
An empty ``ambiguities`` is therefore not a certificate of certainty.
167+
Reporting is deliberately partial: :class:`~nameparser.AmbiguityKind`
168+
lists the forks worth flagging, and even those are not reported
169+
everywhere they occur — the comma paths stay quiet on purpose, since a
170+
comma usually settles the structure before the question arises.
171+
Coverage grows over releases. Treat a non-empty ``ambiguities`` as a
172+
signal to act on; do not read an empty one as a guarantee.
173173

174174
:class:`Tokens <nameparser.Token>` also carry tags — a second, independent label alongside their
175175
role, recording how a token was classified rather than what part of

nameparser/_pipeline/_assign.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ def _assign_main(seg_idx: int, state: ParseState,
129129
# overwrites. The role each ends up with is read back after
130130
# assignment, since which role "not peeled" means depends on
131131
# name_order.
132-
ambiguous_picks: list[tuple[int, ...]] = []
132+
# (piece, cause) -- one kind, two causes, and they need different
133+
# wording: the acronym branch turns on whether periods are written,
134+
# the numeral branch on the letter being a numeral at all
135+
ambiguous_picks: list[tuple[tuple[int, ...], str]] = []
133136
k = len(rest)
134137
while k > 0:
135138
piece = pieces[rest[k - 1]]
@@ -143,7 +146,7 @@ def _assign_main(seg_idx: int, state: ParseState,
143146
# a trailing single letter is a name part unless it happens
144147
# to be a roman numeral -- and V/X/I are ordinary middle
145148
# initials, so taking it as a suffix is a call, not a fact
146-
ambiguous_picks.append(piece)
149+
ambiguous_picks.append((piece, "numeral"))
147150
k -= 1
148151
continue
149152
# A bare ambiguous acronym ("MA", not "M.A.") is a credential
@@ -158,15 +161,15 @@ def _assign_main(seg_idx: int, state: ParseState,
158161
bare_ambiguous = (len(piece) == 1
159162
and "vocab:suffix-ambiguous" in tokens[piece[0]].tags)
160163
if bare_ambiguous and k - 1 >= 2:
161-
ambiguous_picks.append(piece)
164+
ambiguous_picks.append((piece, "acronym"))
162165
k -= 1
163166
continue
164167
if bare_ambiguous and k >= 2:
165168
# not peeled, so it stays the last NAME piece -- which role
166169
# that is depends on name_order, so the detail reads it back
167170
# below. (k < 2 means it is the only piece left, which is
168171
# not the fork this reports.)
169-
ambiguous_picks.append(piece)
172+
ambiguous_picks.append((piece, "acronym"))
170173
break
171174
name_pieces, suffix_pieces = rest[:k], rest[k:]
172175
if not name_pieces and suffix_pieces:
@@ -177,20 +180,26 @@ def _assign_main(seg_idx: int, state: ParseState,
177180
_set_roles(tokens, pieces[piece_idx], roles[pos])
178181
for piece_idx in suffix_pieces:
179182
_set_roles(tokens, pieces[piece_idx], Role.SUFFIX)
180-
for piece in ambiguous_picks:
183+
for piece, cause in ambiguous_picks:
181184
token = tokens[piece[0]]
182185
# every pick was assigned a role just above; the fallback only
183186
# keeps the wording sane if a future path reports before then
184187
role = token.role.value if token.role is not None else "name"
185-
taken, declined = (
186-
("a suffix", "a name part") if token.role is Role.SUFFIX
187-
else (f"a {role} name", "a post-nominal"))
188+
if cause == "numeral":
189+
detail = (
190+
f"{token.text!r} is a roman numeral, so it reads as a "
191+
f"generational suffix; any other single letter there "
192+
f"would be a middle initial")
193+
else:
194+
taken, declined = (
195+
("a suffix", "a name part") if token.role is Role.SUFFIX
196+
else (f"a {role} name", "a post-nominal"))
197+
detail = (
198+
f"{token.text!r} written without periods is both a "
199+
f"post-nominal and an ordinary name; read as {taken} "
200+
f"rather than {declined}")
188201
ambiguities.append(PendingAmbiguity(
189-
AmbiguityKind.SUFFIX_OR_NAME,
190-
f"{token.text!r} written without periods is both a "
191-
f"post-nominal and an ordinary name; read as {taken} "
192-
f"rather than {declined}",
193-
tuple(piece)))
202+
AmbiguityKind.SUFFIX_OR_NAME, detail, tuple(piece)))
194203
# leading ambiguous particle read as a name (#121 surfaced)
195204
if name_pieces:
196205
head = pieces[name_pieces[0]]

nameparser/_types.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,11 +229,11 @@ class AmbiguityKind(StrEnum):
229229
230230
A kind names a FORK THE PARSE HAD TO CALL, not a word that could be
231231
read two ways: the same token elsewhere in a name may present no
232-
choice at all and is then reported by nothing. An empty
233-
``ambiguities`` therefore means none of the forks listed here came
234-
up -- not that the parse was certain of everything. Coverage grows
235-
over releases, so a non-empty tuple is a signal to act on; an empty
236-
one is not a guarantee."""
232+
choice at all and is then reported by nothing. Reporting is also
233+
partial -- a kind listed here is not necessarily emitted everywhere
234+
its fork occurs (the comma paths stay quiet by design), and coverage
235+
grows over releases. A non-empty tuple is a signal to act on; an
236+
empty one is not a guarantee of certainty."""
237237

238238
#: Reserved: the name's field order itself is uncertain (e.g. a
239239
#: two-word name under a non-default name_order). Not yet emitted;

tests/v2/test_parser.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,3 +298,16 @@ def test_chained_particle_detail_does_not_claim_a_role() -> None:
298298
assert n.given == "Van Johnson"
299299
(amb,) = n.ambiguities
300300
assert "family name" not in amb.detail
301+
302+
303+
def test_each_suffix_or_name_branch_describes_itself() -> None:
304+
# one kind, two causes: the acronym branch turns on periods, the
305+
# roman-numeral branch turns on the letter being a numeral. Sharing
306+
# one template made "V written without periods" -- a distinction
307+
# that does not exist for V -- and hid which branch fired.
308+
acronym = parse("John Smith MA").ambiguities[0].detail
309+
assert "periods" in acronym and "post-nominal" in acronym
310+
311+
numeral = parse("John Smith V").ambiguities[0].detail
312+
assert "periods" not in numeral
313+
assert "numeral" in numeral and "initial" in numeral

0 commit comments

Comments
 (0)