Skip to content

Commit 46d4a8d

Browse files
derek73claude
andcommitted
Address PR review findings
- Fix stale parse_pieces docstring that still described adding derived parts "to the constant" - Document the derived-set branch in the is_prefix / is_suffix / is_conjunction docstrings, and reword is_leading_title's now-vacuous "does not mutate C.titles" contrast into the narrower load-bearing fact (the regex match is not registered anywhere, even per-parse) - Remove the redundant _derived_suffixes check from is_suffix_lenient: it falls through to is_suffix(), which already consults the overlay, and derived suffixes always contain an interior period so they can never trip the is_an_initial() veto that "lenient" exists to bypass - Add mutation-verified regression tests for the two coverage gaps: is_rootname's derived-title exclusion (its overlay checks were deletable without any test failing, yet dropping them misparses "Lt.Gov. juan e garcia" into an empty first name) and the __setstate__ backfill for pickles predating the derived sets (whose absence crashes capitalize() with AttributeError) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 61df6fa commit 46d4a8d

3 files changed

Lines changed: 48 additions & 12 deletions

File tree

nameparser/parser.py

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -582,14 +582,16 @@ def is_leading_title(self, piece: str) -> bool:
582582
``is_an_initial()`` check, is what excludes single-letter initials
583583
like ``"J."``. Only meaningful for pieces in the title position
584584
(before the first name is set) — a period-abbreviation appearing
585-
later in the name is left as a middle name. Does not mutate
586-
``C.titles``, so the periodless form (``"Major"``) is never affected
587-
in later parses.
585+
later in the name is left as a middle name. The match is not
586+
registered in ``C.titles`` or the per-parse derived titles, so
587+
matching ``"Major."`` here never makes ``"Major"`` (or ``"Major."``)
588+
a recognized title elsewhere, even within the same parse.
588589
"""
589590
return self.is_title(piece) or bool(self.C.regexes.period_abbreviation.match(piece))
590591

591592
def is_conjunction(self, piece: str) -> bool:
592-
"""Is in the conjunctions set and not :py:func:`is_an_initial()`."""
593+
"""Is in the conjunctions set — config or derived earlier in this
594+
parse (e.g. ``"of the"``) — and not :py:func:`is_an_initial()`."""
593595
if isinstance(piece, list):
594596
for item in piece:
595597
if self.is_conjunction(item):
@@ -602,7 +604,8 @@ def is_conjunction(self, piece: str) -> bool:
602604
def is_prefix(self, piece: str) -> bool:
603605
"""
604606
Lowercased, leading/trailing-periods-stripped version of piece is in the
605-
:py:data:`~nameparser.config.prefixes.PREFIXES` set.
607+
:py:data:`~nameparser.config.prefixes.PREFIXES` set, or was derived as
608+
a prefix earlier in this parse (e.g. ``"von und"``).
606609
"""
607610
if isinstance(piece, list):
608611
for item in piece:
@@ -657,7 +660,9 @@ def is_roman_numeral(self, value: str) -> bool:
657660

658661
def is_suffix(self, piece: str) -> bool:
659662
"""
660-
Is in the suffixes set and not :py:func:`is_an_initial()`.
663+
Is in the suffixes set — or was derived as a period-joined suffix
664+
earlier in this parse (e.g. ``"JD.CPA"``) — and not
665+
:py:func:`is_an_initial()`.
661666
662667
Some suffixes may be acronyms (M.B.A) while some are not (Jr.),
663668
so we remove the periods from `piece` when testing against
@@ -697,10 +702,7 @@ def is_suffix_lenient(self, piece: str) -> bool:
697702
is_suffix() would otherwise reject. Only safe for pieces in
698703
unambiguous positions, e.g. after a comma ("John Ingram, V").
699704
"""
700-
word = lc(piece)
701-
return word in self.C.suffix_not_acronyms \
702-
or word in self._derived_suffixes \
703-
or self.is_suffix(piece)
705+
return lc(piece) in self.C.suffix_not_acronyms or self.is_suffix(piece)
704706

705707
def expand_suffix_delimiter(self, part: str) -> list[str]:
706708
"""Split a single post-comma part on :py:attr:`suffix_delimiter`,
@@ -1211,8 +1213,9 @@ def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) ->
12111213
lastname prefixes. Tokens that are empty after stripping spaces and
12121214
commas are dropped, so the returned pieces never contain empty
12131215
strings. If parts have periods in the middle, try splitting
1214-
on periods and check if the parts are titles or suffixes. If they are
1215-
add to the constant so they will be found.
1216+
on periods and check if the parts are titles or suffixes. If they are,
1217+
register the periods-joined part as a derived title/suffix for this
1218+
parse so it will be recognized; the constants are not modified.
12161219
12171220
:param list parts: name part strings from the comma split
12181221
:param int additional_parts_count:

tests/test_python_api.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,27 @@ def test_name_instance_deepcopy_isolates_instance_config(self) -> None:
9595
self.assertIn('chancellor', dup.C.titles)
9696
self.assertNotIn('marker', hn.C.titles)
9797

98+
def test_unpickle_legacy_state_without_derived_sets(self) -> None:
99+
"""Pickles from before the per-parse derived sets existed must still work.
100+
101+
Their state lacks the ``_derived_*`` attributes, which the ``is_*``
102+
predicates (and through them ``capitalize()``) read directly, so
103+
``__setstate__`` must backfill them rather than crash with
104+
AttributeError on first use.
105+
"""
106+
hn = HumanName("dr. juan de la vega jr.")
107+
legacy_state = {
108+
k: v for k, v in hn.__getstate__().items()
109+
if not k.startswith('_derived_')
110+
}
111+
112+
restored = HumanName.__new__(HumanName)
113+
restored.__setstate__(legacy_state)
114+
115+
self.assertTrue(restored.is_title('dr.'))
116+
restored.capitalize() # reads _derived_prefixes via cap_word/is_prefix
117+
self.assertEqual(str(restored), "Dr. Juan de la Vega Jr.")
118+
98119
def test_pickle_default_name_preserves_singleton_identity(self) -> None:
99120
"""A default HumanName must re-attach to CONSTANTS after a pickle round-trip.
100121

tests/test_titles.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,18 @@ def test_title_with_periods_lastname_comma(self) -> None:
225225
self.m(hn.first, "John", hn)
226226
self.m(hn.last, "Doe", hn)
227227

228+
def test_title_with_periods_and_single_letter_middle_name(self) -> None:
229+
# A derived title ("Lt.Gov.") must be excluded from the rootname
230+
# count that join_on_conjunctions() uses for its single-letter
231+
# conjunction heuristic. If is_rootname() misses the derived titles,
232+
# the count reaches 4 and "e" is treated as a conjunction, joining
233+
# "juan e garcia" into a single last-name piece with no first name.
234+
hn = HumanName("Lt.Gov. juan e garcia")
235+
self.m(hn.title, "Lt.Gov.", hn)
236+
self.m(hn.first, "juan", hn)
237+
self.m(hn.middle, "e", hn)
238+
self.m(hn.last, "garcia", hn)
239+
228240
def test_mac_with_spaces(self) -> None:
229241
hn = HumanName("Jane Mac Beth")
230242
self.m(hn.first, "Jane", hn)

0 commit comments

Comments
 (0)