Skip to content

Commit 37d5c69

Browse files
derek73claude
andcommitted
Simplify pass over the v1.2.1..HEAD changes
Behavior-preserving cleanups from a four-angle review (reuse, simplification, efficiency, altitude) of everything since v1.2.1: - join_on_conjunctions: share one register_joined_piece helper for the title/prefix registration duplicated across both join branches - Consolidate the lenient post-comma suffix rule into is_suffix_lenient(); are_suffixes_after_comma uses it, and the single-use is_suffix_at_lastname_comma_end method is inlined at its only call site - cap_word: compute the exception-lookup key once instead of up to four lc()/replace() calls per word - Constants.__setstate__: drop the verification loop that could never fire for state produced by __getstate__ - Extract _is_dunder() for the guard copy-pasted across four TupleManager/RegexTupleManager attribute hooks - tests: add FlaggedConstantsTestBase replacing seven copy-pasted setup_method/hn() fixture pairs; drop the two invariant tests that duplicate prefixes.py's import-time asserts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f3f6fa3 commit 37d5c69

9 files changed

Lines changed: 111 additions & 145 deletions

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_
134134

135135
**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)
136136

137-
**`suffix_not_acronyms` vs `is_an_initial` tension** — single-letter roman numeral suffixes (`i`, `v`) are in `suffix_not_acronyms` but also match the `is_an_initial` regex (single uppercase letter), so `is_suffix()` rejects them. Two separate code paths need context-aware workarounds: (1) suffix-comma detection uses `are_suffixes_after_comma()` which bypasses `is_suffix()` for `suffix_not_acronyms` members; (2) lastname-comma post-comma parsing uses `is_suffix_at_lastname_comma_end()` which only fires when `nxt is None` and `len(parts)==2` (no `parts[2]` suffix segment). See issues #136, #144.
137+
**`suffix_not_acronyms` vs `is_an_initial` tension** — single-letter roman numeral suffixes (`i`, `v`) are in `suffix_not_acronyms` but also match the `is_an_initial` regex (single uppercase letter), so `is_suffix()` rejects them. The lenient test lives in `is_suffix_lenient()`, which accepts `suffix_not_acronyms` members unconditionally and is only safe in unambiguous positions: (1) suffix-comma detection uses it via `are_suffixes_after_comma()`; (2) lastname-comma post-comma parsing uses it inline, only when `nxt is None` and `len(parts)==2` (no `parts[2]` suffix segment). See issues #136, #144.
138138

139139
**Expected-failure tests use `@pytest.mark.xfail`** — the conftest parametrized fixture breaks `@unittest.expectedFailure`; always use `@pytest.mark.xfail` instead.
140140

nameparser/config/__init__.py

Lines changed: 28 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -140,40 +140,47 @@ def clear(self) -> Self:
140140
T = TypeVar('T')
141141

142142

143+
def _is_dunder(attr: str) -> bool:
144+
# Dunder names are Python's protocol probes (copy looks up __deepcopy__,
145+
# inspect.unwrap looks up __wrapped__, typing's GenericAlias.__call__ sets
146+
# __orig_class__, ...), never config keys. The TupleManager attribute hooks
147+
# all route dunders to normal object-attribute behavior so those probes
148+
# work instead of being mistaken for dict entries.
149+
return attr.startswith("__") and attr.endswith("__")
150+
151+
143152
class TupleManager(dict[str, T]):
144153
'''
145-
A dictionary with dot.notation access. Subclass of ``dict``. Makes the tuple constants
154+
A dictionary with dot.notation access. Subclass of ``dict``. Makes the tuple constants
146155
more friendly.
147156
'''
148157

149158
def __getattr__(self, attr: str) -> T | None:
150-
# Dunder names are Python's protocol probes (copy looks up __deepcopy__,
151-
# inspect.unwrap looks up __wrapped__, ...), never config keys. Report
152-
# them as genuinely absent so hasattr() is honest and those probes work;
153-
# otherwise the dict default is mistaken for a real protocol hook. See
154-
# RegexTupleManager.__getattr__ for the concrete failure this prevents.
155-
if attr.startswith("__") and attr.endswith("__"):
159+
# Report dunders as genuinely absent so hasattr() is honest and
160+
# protocol probes work; otherwise the dict default is mistaken for a
161+
# real protocol hook. See RegexTupleManager.__getattr__ for the
162+
# concrete failure this prevents.
163+
if _is_dunder(attr):
156164
raise AttributeError(attr)
157165
return self.get(attr)
158166

159167
def __setattr__(self, attr: str, value: T) -> None:
160-
# Dunder names are Python's protocol probes, not config keys -- same
161-
# rationale as __getattr__ above. Concretely: constructing a
162-
# subscripted generic, e.g. TupleManager[re.Pattern[str] | str](...),
163-
# makes typing's GenericAlias.__call__ set `__orig_class__` on the new
164-
# instance right after __init__ returns. Without this guard that
165-
# assignment falls through to dict.__setitem__ and silently inserts a
166-
# bogus '__orig_class__' entry into the dict itself, corrupting
167-
# .values()/iteration. Fall back to normal object attribute storage
168-
# for dunders; everything else keeps the dict-backed dot-notation
169-
# behavior this class exists for.
170-
if attr.startswith("__") and attr.endswith("__"):
168+
# Fall back to normal object attribute storage for dunders; everything
169+
# else keeps the dict-backed dot-notation behavior this class exists
170+
# for. Concretely: constructing a subscripted generic, e.g.
171+
# TupleManager[re.Pattern[str] | str](...), makes typing's
172+
# GenericAlias.__call__ set `__orig_class__` on the new instance right
173+
# after __init__ returns. Without this guard that assignment falls
174+
# through to dict.__setitem__ and silently inserts a bogus
175+
# '__orig_class__' entry into the dict itself, corrupting
176+
# .values()/iteration.
177+
if _is_dunder(attr):
171178
object.__setattr__(self, attr, value)
172179
else:
173180
self[attr] = value
174181

175182
def __delattr__(self, attr: str) -> None:
176-
if attr.startswith("__") and attr.endswith("__"):
183+
if _is_dunder(attr):
177184
object.__delattr__(self, attr)
178185
else:
179186
del self[attr]
@@ -194,12 +201,10 @@ def __reduce__(self) -> tuple[type, tuple[()], Mapping[str, T]]:
194201

195202
class RegexTupleManager(TupleManager[re.Pattern[str]]):
196203
def __getattr__(self, attr: str) -> re.Pattern[str]:
197-
# Dunder names are Python's protocol probes (copy.deepcopy looks up
198-
# __deepcopy__, inspect.unwrap looks up __wrapped__, ...), never regex
199-
# keys. Report them as genuinely absent; otherwise the EMPTY_REGEX
204+
# Report dunders as genuinely absent; otherwise the EMPTY_REGEX
200205
# default is mistaken for a real protocol hook — e.g. copy.deepcopy
201206
# tries to call the returned re.Pattern and raises TypeError.
202-
if attr.startswith("__") and attr.endswith("__"):
207+
if _is_dunder(attr):
203208
raise AttributeError(attr)
204209
return self.get(attr, EMPTY_REGEX)
205210

@@ -536,16 +541,6 @@ def __setstate__(self, state: Mapping[str, Any]) -> None:
536541
self._pst = None
537542
for name, value in state.items():
538543
setattr(self, name, value)
539-
# Verify each descriptor-backed attr was restored. Without this, a missing
540-
# key surfaces later as AttributeError: 'Constants' object has no attribute
541-
# '_prefixes' — the private mangled name, not the public one, making it
542-
# very hard to diagnose.
543-
for attr in (n for n, v in vars(type(self)).items() if isinstance(v, _CachedUnionMember)):
544-
if not hasattr(self, '_' + attr):
545-
raise ValueError(
546-
f"Pickle state is missing required field {attr!r}. "
547-
"The state blob may be truncated or from an incompatible version."
548-
)
549544

550545
def __getstate__(self) -> Mapping[str, Any]:
551546
# Pickle the instance's own configuration: the collections built in

nameparser/parser.py

Lines changed: 40 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -663,43 +663,24 @@ def are_suffixes(self, pieces: Iterable[str]) -> bool:
663663
return False
664664
return True
665665

666-
def are_suffixes_after_comma(self, pieces: Iterable[str]) -> bool:
667-
"""Like are_suffixes, but pieces found in suffix_not_acronyms are
668-
accepted unconditionally without passing through is_suffix().
666+
def is_suffix_lenient(self, piece: str) -> bool:
667+
"""Like is_suffix(), but suffix_not_acronyms members are accepted
668+
unconditionally, bypassing is_suffix()'s is_an_initial() veto.
669669
670-
Used when detecting suffix-comma format (e.g. "John Ingram, V") where
671-
the post-comma position is unambiguous. This covers all
672-
suffix_not_acronyms members (i, ii, iii, iv, v, jr, sr, etc.),
673-
case-insensitively, including single-letter entries that is_suffix()
674-
would otherwise reject via is_an_initial().
670+
This covers all suffix_not_acronyms members (i, ii, iii, iv, v, jr,
671+
sr, etc.), case-insensitively, including single-letter entries that
672+
is_suffix() would otherwise reject. Only safe for pieces in
673+
unambiguous positions, e.g. after a comma ("John Ingram, V").
675674
"""
676-
for piece in pieces:
677-
if lc(piece) in self.C.suffix_not_acronyms:
678-
continue
679-
if not self.is_suffix(piece):
680-
return False
681-
return True
682-
683-
def is_suffix_at_lastname_comma_end(self, piece: str, nxt: str | None, parts: list[str]) -> bool:
684-
"""True when ``piece`` is a suffix_not_acronyms member that should be
685-
treated as a suffix at the end of ``parts[1]`` (the post-comma segment)
686-
in a lastname-comma name, where ``parts`` is the full comma-split of the
687-
name string.
688-
689-
Returns True only when all three conditions hold:
690-
- ``nxt is None``: piece is the last token in the post-comma segment
691-
- ``len(parts) == 2``: no ``parts[2]`` suffix segment exists
692-
- ``lc(piece) in suffix_not_acronyms``
675+
return lc(piece) in self.C.suffix_not_acronyms or self.is_suffix(piece)
693676

694-
When ``parts[2]`` exists the caller already declared an explicit suffix
695-
via comma (e.g. 'Doe, Rev. John V, Jr.'), making the trailing token more
696-
likely a middle initial; ``len(parts) == 2`` excludes that case.
697-
Used as an OR alternative to ``is_suffix()`` for pieces that
698-
``is_suffix()`` would reject via ``is_an_initial()``.
677+
def are_suffixes_after_comma(self, pieces: Iterable[str]) -> bool:
678+
"""Return True if all pieces are suffixes by the lenient
679+
:py:func:`is_suffix_lenient` test. Used when detecting suffix-comma
680+
format (e.g. "John Ingram, V") where the post-comma position is
681+
unambiguous.
699682
"""
700-
return (nxt is None
701-
and len(parts) == 2
702-
and lc(piece) in self.C.suffix_not_acronyms)
683+
return all(self.is_suffix_lenient(piece) for piece in pieces)
703684

704685
def is_rootname(self, piece: str) -> bool:
705686
"""
@@ -1147,7 +1128,16 @@ def parse_full_name(self) -> None:
11471128
if not self.first:
11481129
self.first_list.append(piece)
11491130
continue
1150-
if self.is_suffix(piece) or self.is_suffix_at_lastname_comma_end(piece, nxt, parts):
1131+
# A trailing token in a two-part lastname-comma name is
1132+
# unambiguously positioned, so use the lenient test that
1133+
# accepts suffix_not_acronyms members is_suffix() would
1134+
# veto as initials. When parts[2] exists the caller
1135+
# already declared an explicit suffix via comma (e.g.
1136+
# 'Doe, Rev. John V, Jr.'), making the trailing token
1137+
# more likely a middle initial.
1138+
if self.is_suffix(piece) or \
1139+
(nxt is None and len(parts) == 2
1140+
and self.is_suffix_lenient(piece)):
11511141
self.suffix_list.append(piece)
11521142
continue
11531143
self.middle_list.append(piece)
@@ -1266,6 +1256,16 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
12661256
# refresh conjunction index locations
12671257
conj_index = [i for i, piece in enumerate(pieces) if self.is_conjunction(piece)]
12681258

1259+
def register_joined_piece(new_piece: str, neighbor: str) -> None:
1260+
if self.is_title(neighbor):
1261+
# when joining to a title, make new_piece a title too
1262+
self.C.titles.add(new_piece)
1263+
if self.is_prefix(neighbor):
1264+
# when joining to a prefix, make new_piece a prefix too, so
1265+
# e.g. "von" + "und" bridges into "von und" and can still
1266+
# chain onto a following prefix/lastname (see "von und zu")
1267+
self.C.prefixes.add(new_piece)
1268+
12691269
for i in conj_index:
12701270
if len(pieces[i]) == 1 and total_length < 4 and pieces[i].isalpha():
12711271
# if there are only 3 total parts (minus known titles, suffixes
@@ -1276,14 +1276,7 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
12761276

12771277
if i == 0:
12781278
new_piece = " ".join(pieces[i:i+2])
1279-
if self.is_title(pieces[i+1]):
1280-
# when joining to a title, make new_piece a title too
1281-
self.C.titles.add(new_piece)
1282-
if self.is_prefix(pieces[i+1]):
1283-
# when joining to a prefix, make new_piece a prefix too, so
1284-
# e.g. "von" + "und" bridges into "von und" and can still
1285-
# chain onto a following prefix/lastname (see "von und zu")
1286-
self.C.prefixes.add(new_piece)
1279+
register_joined_piece(new_piece, pieces[i+1])
12871280
pieces[i] = new_piece
12881281
pieces.pop(i+1)
12891282
# subtract 1 from the index of all the remaining conjunctions
@@ -1293,14 +1286,7 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
12931286

12941287
else:
12951288
new_piece = " ".join(pieces[i-1:i+2])
1296-
if self.is_title(pieces[i-1]):
1297-
# when joining to a title, make new_piece a title too
1298-
self.C.titles.add(new_piece)
1299-
if self.is_prefix(pieces[i-1]):
1300-
# when joining to a prefix, make new_piece a prefix too, so
1301-
# e.g. "von" + "und" bridges into "von und" and can still
1302-
# chain onto a following prefix/lastname (see "von und zu")
1303-
self.C.prefixes.add(new_piece)
1289+
register_joined_piece(new_piece, pieces[i-1])
13041290
pieces[i-1] = new_piece
13051291
pieces.pop(i)
13061292
rm_count = 2
@@ -1372,10 +1358,10 @@ def cap_word(self, word: str, attribute: HumanNameAttributeT) -> str:
13721358
or self.is_conjunction(word):
13731359
return word.lower()
13741360
exceptions = self.C.capitalization_exceptions
1375-
if lc(word) in exceptions:
1376-
return exceptions[lc(word)]
1377-
if lc(word).replace('.', '') in exceptions:
1378-
return exceptions[lc(word).replace('.', '')]
1361+
key = lc(word)
1362+
for k in (key, key.replace('.', '')):
1363+
if k in exceptions:
1364+
return exceptions[k]
13791365
mac_match = self.C.regexes.mac.match(word)
13801366
if mac_match:
13811367
def cap_after_mac(m: re.Match) -> str:

tests/base.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
from typing import Generic, TypeVar
1+
from typing import Any, ClassVar, Generic, TypeVar
22

33
from nameparser import HumanName
4+
from nameparser.config import Constants
45

56
T = TypeVar('T')
67

@@ -51,3 +52,20 @@ def assertIsNone(self, expr: object, msg: object = None) -> None:
5152

5253
def assertIsNotNone(self, expr: object, msg: object = None) -> None:
5354
assert expr is not None, msg or "unexpectedly None"
55+
56+
57+
class FlaggedConstantsTestBase(HumanNameTestBase[T]):
58+
"""Base for test classes that parse with a dedicated, flagged Constants.
59+
60+
Subclasses set ``constants_kwargs``; each test method gets a fresh
61+
``Constants(**constants_kwargs)`` via ``setup_method``, and ``hn()``
62+
parses with it.
63+
"""
64+
65+
constants_kwargs: ClassVar[dict[str, Any]] = {}
66+
67+
def setup_method(self) -> None:
68+
self.C = Constants(**self.constants_kwargs)
69+
70+
def hn(self, name: str) -> HumanName:
71+
return HumanName(name, constants=self.C)

tests/test_east_slavic_patronymic_order.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from nameparser import HumanName
22
from nameparser.config import Constants
3-
from tests.base import HumanNameTestBase
3+
from tests.base import FlaggedConstantsTestBase, HumanNameTestBase
44

55

66
def test_latin_patronymic_matches() -> None:
@@ -49,14 +49,10 @@ def test_cyrillic_patronymic_rejects_non_patronymic() -> None:
4949
assert not C.regexes.east_slavic_patronymic_cyrillic.search("Иванов")
5050

5151

52-
class PatronymicNameOrderReorderTests(HumanNameTestBase):
52+
class PatronymicNameOrderReorderTests(FlaggedConstantsTestBase):
5353
"""Names that SHOULD be rotated when the flag is on."""
5454

55-
def setup_method(self) -> None:
56-
self.C = Constants(patronymic_name_order=True)
57-
58-
def hn(self, name: str) -> HumanName:
59-
return HumanName(name, constants=self.C)
55+
constants_kwargs = {"patronymic_name_order": True}
6056

6157
def test_canonical_latin(self) -> None:
6258
n = self.hn("Ivanov Ivan Ivanovich")
@@ -121,14 +117,10 @@ def test_western_patronymic_surname_reordered_when_flag_on(self) -> None:
121117
assert n.last == "David"
122118

123119

124-
class PatronymicNameOrderGuardsTests(HumanNameTestBase):
120+
class PatronymicNameOrderGuardsTests(FlaggedConstantsTestBase):
125121
"""Names that must NOT be reordered even when the flag is on."""
126122

127-
def setup_method(self) -> None:
128-
self.C = Constants(patronymic_name_order=True)
129-
130-
def hn(self, name: str) -> HumanName:
131-
return HumanName(name, constants=self.C)
123+
constants_kwargs = {"patronymic_name_order": True}
132124

133125
def test_already_correct_order(self) -> None:
134126
# middle is patronymic → already in Western order, do not rotate

tests/test_middle_name_as_last.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from nameparser import HumanName
22
from nameparser.config import Constants
3-
from tests.base import HumanNameTestBase
3+
from tests.base import FlaggedConstantsTestBase, HumanNameTestBase
44

55

66
class MiddleNameAsLastFlagTests(HumanNameTestBase):
@@ -20,13 +20,9 @@ def test_does_not_affect_other_instance(self) -> None:
2020
assert C2.middle_name_as_last is False
2121

2222

23-
class MiddleNameAsLastFoldTests(HumanNameTestBase):
23+
class MiddleNameAsLastFoldTests(FlaggedConstantsTestBase):
2424

25-
def setup_method(self) -> None:
26-
self.C = Constants(middle_name_as_last=True)
27-
28-
def hn(self, name: str) -> HumanName:
29-
return HumanName(name, constants=self.C)
25+
constants_kwargs = {"middle_name_as_last": True}
3026

3127
def test_fold_no_comma(self) -> None:
3228
n = self.hn("Mohamad Ahmad Ali Hassan")
@@ -98,17 +94,13 @@ def test_default_constants_unaffected(self) -> None:
9894
self.m(n.last, "Hassan", n)
9995

10096

101-
class MiddleNameAsLastWithPatronymicOrderTests(HumanNameTestBase):
97+
class MiddleNameAsLastWithPatronymicOrderTests(FlaggedConstantsTestBase):
10298
"""Both localization flags on: patronymic reordering must settle
10399
first/middle/last before the fold collapses middle into last, per the
104100
design's stated ordering rationale (post_process() runs the patronymic
105101
hook before the middle_name_as_last hook)."""
106102

107-
def setup_method(self) -> None:
108-
self.C = Constants(middle_name_as_last=True, patronymic_name_order=True)
109-
110-
def hn(self, name: str) -> HumanName:
111-
return HumanName(name, constants=self.C)
103+
constants_kwargs = {"middle_name_as_last": True, "patronymic_name_order": True}
112104

113105
def test_rotate_then_fold_no_comma(self) -> None:
114106
# patronymic_name_order rotates "Ivanov Petr Sergeyevich" to

0 commit comments

Comments
 (0)