Skip to content

Commit 61df6fa

Browse files
derek73claude
andcommitted
Stop parsing from mutating the Constants it reads
parse_pieces() and join_on_conjunctions() derived new lookup entries while parsing (period-joined titles/suffixes like "Lt.Gov.", conjunction-joined pieces like "Mr. and Mrs." and "von und zu") and add()ed them directly to self.C — by default the shared module-level CONSTANTS singleton. Parsing one name therefore permanently changed how every later name in the process parsed: results depended on input order, were not reproducible across runs, and concurrent parsing raced on the shared sets. It also churned the suffixes_prefixes_titles cache, which invalidated on every such add(). The derivations are only needed within the parse that produced them, so track them in per-instance _derived_* sets that the is_title / is_suffix / is_conjunction / is_prefix / is_rootname predicates consult alongside self.C, and reset them at the start of each parse_full_name() run. Parse results are unchanged (each parse re-derives its own entries); the config is now read-only during parsing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 016873c commit 61df6fa

3 files changed

Lines changed: 136 additions & 21 deletions

File tree

docs/release_log.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
Release Log
22
===========
33
* 1.3.0 - Unreleased
4+
- Fix parsing writing back into the ``Constants`` it reads (usually the shared module-level ``CONSTANTS``): pieces derived while parsing a name — period-joined titles/suffixes like ``"Lt.Gov."`` and conjunction-joined pieces like ``"Mr. and Mrs."`` or ``"von und zu"`` — are now tracked per parse instead of being permanently ``add()``-ed to the config, so parse results no longer depend on which names were parsed earlier in the process and parsing no longer mutates shared state across threads
45
- Remove the vestigial ``unparsable`` attribute: the guard that was meant to set it has been unreachable since 2013 (v0.2.9), so it has reported ``False`` for every parsed name for over a decade; check ``len(name) == 0`` to detect an empty parse
56
- Fix ``__hash__`` to lowercase the name like ``__eq__`` does, so equal ``HumanName`` instances hash equal and behave correctly in sets and dicts
67
- Fix ``initials()`` emitting a stray empty initial (e.g. ``"J. . V."``) -- or raising ``TypeError`` when ``empty_attribute_default`` is ``None`` -- for name parts with no initialable words, e.g. a prefix-only middle name like ``"de la"``

nameparser/parser.py

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,19 @@ def __init__(
115115
if type(self.C) is not type(CONSTANTS):
116116
self.C = Constants()
117117

118+
# Lookup entries derived while parsing this instance (period-joined
119+
# titles/suffixes like "Lt.Gov.", conjunction-joined pieces like
120+
# "Mr. and Mrs." or "von und zu"). Kept separate from self.C so that
121+
# parsing never writes into the config — which is usually the shared
122+
# module-level CONSTANTS — keeping results independent of what was
123+
# parsed before and config reads safe across threads. Values are
124+
# lc()-normalized, mirroring how SetManager stores them. Reset at the
125+
# start of each parse_full_name() run.
126+
self._derived_titles: set[str] = set()
127+
self._derived_suffixes: set[str] = set()
128+
self._derived_conjunctions: set[str] = set()
129+
self._derived_prefixes: set[str] = set()
130+
118131
self.encoding = encoding
119132
self.string_format = string_format if string_format is not None else self.C.string_format
120133
self.initials_format = initials_format if initials_format is not None else self.C.initials_format
@@ -144,6 +157,11 @@ def __setstate__(self, state: dict) -> None:
144157
if state.get('C') is None:
145158
state['C'] = CONSTANTS
146159
self.__dict__.update(state)
160+
# pickles from before the per-parse derived sets existed lack them;
161+
# backfill so the is_* predicates work without a re-parse
162+
for attr in ('_derived_titles', '_derived_suffixes',
163+
'_derived_conjunctions', '_derived_prefixes'):
164+
self.__dict__.setdefault(attr, set())
147165

148166
def __iter__(self) -> Iterator[str]:
149167
return self
@@ -550,8 +568,11 @@ def _set_list(self, attr: str, value: str | list[str] | None) -> None:
550568

551569
# Parse helpers
552570
def is_title(self, value: str) -> bool:
553-
"""Is in the :py:data:`~nameparser.config.titles.TITLES` set."""
554-
return lc(value) in self.C.titles
571+
"""Is in the :py:data:`~nameparser.config.titles.TITLES` set or was
572+
derived as a title earlier in this parse (e.g. ``"Lt.Gov."``,
573+
``"Mr. and Mrs."``)."""
574+
word = lc(value)
575+
return word in self.C.titles or word in self._derived_titles
555576

556577
def is_leading_title(self, piece: str) -> bool:
557578
"""
@@ -574,7 +595,9 @@ def is_conjunction(self, piece: str) -> bool:
574595
if self.is_conjunction(item):
575596
return True
576597
return False
577-
return piece.lower() in self.C.conjunctions and not self.is_an_initial(piece)
598+
return (piece.lower() in self.C.conjunctions
599+
or piece.lower() in self._derived_conjunctions) \
600+
and not self.is_an_initial(piece)
578601

579602
def is_prefix(self, piece: str) -> bool:
580603
"""
@@ -586,7 +609,8 @@ def is_prefix(self, piece: str) -> bool:
586609
if self.is_prefix(item):
587610
return True
588611
return False
589-
return lc(piece) in self.C.prefixes
612+
word = lc(piece)
613+
return word in self.C.prefixes or word in self._derived_prefixes
590614

591615
def is_bound_first_name(self, piece: str) -> bool:
592616
"""Lowercased, leading/trailing-periods-stripped version of piece is in :py:attr:`~nameparser.config.Constants.bound_first_names`."""
@@ -646,8 +670,10 @@ def is_suffix(self, piece: str) -> bool:
646670
return True
647671
return False
648672
else:
649-
return ((lc(piece).replace('.', '') in self.C.suffix_acronyms)
650-
or (lc(piece) in self.C.suffix_not_acronyms)) \
673+
word = lc(piece)
674+
return ((word.replace('.', '') in self.C.suffix_acronyms)
675+
or (word in self.C.suffix_not_acronyms)
676+
or (word in self._derived_suffixes)) \
651677
and not self.is_an_initial(piece)
652678

653679
def are_suffixes(self, pieces: Iterable[str]) -> bool:
@@ -671,7 +697,10 @@ def is_suffix_lenient(self, piece: str) -> bool:
671697
is_suffix() would otherwise reject. Only safe for pieces in
672698
unambiguous positions, e.g. after a comma ("John Ingram, V").
673699
"""
674-
return lc(piece) in self.C.suffix_not_acronyms or self.is_suffix(piece)
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)
675704

676705
def expand_suffix_delimiter(self, part: str) -> list[str]:
677706
"""Split a single post-comma part on :py:attr:`suffix_delimiter`,
@@ -696,7 +725,11 @@ def is_rootname(self, piece: str) -> bool:
696725
"""
697726
Is not a known title, suffix or prefix. Just first, middle, last names.
698727
"""
699-
return lc(piece) not in self.C.suffixes_prefixes_titles \
728+
word = lc(piece)
729+
return word not in self.C.suffixes_prefixes_titles \
730+
and word not in self._derived_titles \
731+
and word not in self._derived_suffixes \
732+
and word not in self._derived_prefixes \
700733
and not self.is_an_initial(piece)
701734

702735
def is_an_initial(self, value: str) -> bool:
@@ -1005,6 +1038,13 @@ def parse_full_name(self) -> None:
10051038
self.nickname_list = []
10061039
self.maiden_list = []
10071040

1041+
# each parse derives these from scratch; entries from a previous
1042+
# full_name must not influence this one
1043+
self._derived_titles = set()
1044+
self._derived_suffixes = set()
1045+
self._derived_conjunctions = set()
1046+
self._derived_prefixes = set()
1047+
10081048
self.pre_process()
10091049

10101050
self._full_name = self.collapse_whitespace(self._full_name)
@@ -1195,8 +1235,8 @@ def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) ->
11951235
output += [s for s in (x.strip(' ,') for x in part.split(' ')) if s]
11961236

11971237
# If part contains periods, check if it's multiple titles or suffixes
1198-
# together without spaces if so, add the new part with periods to the
1199-
# constants so they get parsed correctly later
1238+
# together without spaces. If so, register the periods-joined part as
1239+
# a derived title/suffix for this parse so it gets recognized later
12001240
for part in output:
12011241
# if this part has a period not at the beginning or end
12021242
if self.C.regexes.period_not_at_end and self.C.regexes.period_not_at_end.match(part):
@@ -1206,12 +1246,12 @@ def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) ->
12061246
titles = list(filter(self.is_title, period_chunks))
12071247
suffixes = list(filter(self.is_suffix, period_chunks))
12081248

1209-
# add the part to the constant so it will be found
1249+
# register the part so it will be found by the is_* checks
12101250
if len(list(titles)):
1211-
self.C.titles.add(part)
1251+
self._derived_titles.add(lc(part))
12121252
continue
12131253
if len(list(suffixes)):
1214-
self.C.suffix_not_acronyms.add(part)
1254+
self._derived_suffixes.add(lc(part))
12151255
continue
12161256

12171257
return self.join_on_conjunctions(output, additional_parts_count)
@@ -1226,10 +1266,11 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
12261266
['The', 'Secretary', 'of', 'State', 'Hillary', 'Clinton'] ==>
12271267
['The Secretary of State', 'Hillary', 'Clinton']
12281268
1229-
When joining titles, saves newly formed piece to the instance's titles
1230-
constant so they will be parsed correctly later. E.g. after parsing the
1231-
example names above, 'The Secretary of State' and 'Mr. and Mrs.' would
1232-
be present in the titles constant set.
1269+
When joining titles, registers the newly formed piece as a derived
1270+
title for the current parse so it will be recognized correctly later
1271+
in the same parse. E.g. while parsing the example names above,
1272+
'The Secretary of State' and 'Mr. and Mrs.' are treated as titles.
1273+
The configuration in ``self.C`` is never modified.
12331274
12341275
:param list pieces: name pieces strings after split on spaces
12351276
:param int additional_parts_count:
@@ -1259,8 +1300,8 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
12591300
for cont_i in reversed(contiguous_conj_i):
12601301
new_piece = " ".join(pieces[cont_i[0]: cont_i[1]+1])
12611302
pieces[cont_i[0]:cont_i[1]+1] = [new_piece]
1262-
# add newly joined conjunctions to constants to be found later
1263-
self.C.conjunctions.add(new_piece)
1303+
# register newly joined conjunctions to be found later this parse
1304+
self._derived_conjunctions.add(lc(new_piece))
12641305

12651306
if len(pieces) == 1:
12661307
# if there's only one piece left, nothing left to do
@@ -1272,12 +1313,12 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
12721313
def register_joined_piece(new_piece: str, neighbor: str) -> None:
12731314
if self.is_title(neighbor):
12741315
# when joining to a title, make new_piece a title too
1275-
self.C.titles.add(new_piece)
1316+
self._derived_titles.add(lc(new_piece))
12761317
if self.is_prefix(neighbor):
12771318
# when joining to a prefix, make new_piece a prefix too, so
12781319
# e.g. "von" + "und" bridges into "von und" and can still
12791320
# chain onto a following prefix/lastname (see "von und zu")
1280-
self.C.prefixes.add(new_piece)
1321+
self._derived_prefixes.add(lc(new_piece))
12811322

12821323
def shift_conj_index(past: int, by: int) -> None:
12831324
# after removing pieces at/after `past`, indices of the

tests/test_constants.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,79 @@ def test_setstate_raises_on_missing_descriptor_field(self) -> None:
487487
restored.__setstate__(state)
488488

489489

490+
class ParsingDoesNotMutateConfigTests(HumanNameTestBase):
491+
"""Parsing a name must never write back into the Constants it reads.
492+
493+
The parser derives extra lookup entries while parsing (period-joined
494+
titles/suffixes like "Lt.Gov.", conjunction-joined pieces like
495+
"Mr. and Mrs." or "von und zu"). Those derivations are needed within the
496+
parse, but historically they were add()ed to ``self.C`` — by default the
497+
shared module-level CONSTANTS singleton — so parsing one name permanently
498+
changed how every later name in the process parsed: parse results depended
499+
on input order, and concurrent parsing raced on the shared sets.
500+
"""
501+
502+
_WATCHED_ATTRS = ('titles', 'prefixes', 'conjunctions',
503+
'suffix_acronyms', 'suffix_not_acronyms')
504+
505+
def _assert_parse_leaves_config_unchanged(self, name: str) -> HumanName:
506+
from nameparser.config import CONSTANTS
507+
before = {a: set(getattr(CONSTANTS, a)) for a in self._WATCHED_ATTRS}
508+
hn = HumanName(name)
509+
for attr, snapshot in before.items():
510+
leaked = set(getattr(CONSTANTS, attr)) - snapshot
511+
self.assertEqual(leaked, set(),
512+
f"parsing {name!r} leaked {leaked!r} into CONSTANTS.{attr}")
513+
return hn
514+
515+
def test_period_joined_title_does_not_leak_into_titles(self) -> None:
516+
hn = self._assert_parse_leaves_config_unchanged("Lt.Gov. John Doe")
517+
# the within-parse derivation must still work
518+
self.m(hn.title, "Lt.Gov.", hn)
519+
self.m(hn.first, "John", hn)
520+
self.m(hn.last, "Doe", hn)
521+
522+
def test_period_joined_suffix_does_not_leak_into_suffixes(self) -> None:
523+
hn = self._assert_parse_leaves_config_unchanged("John Doe JD.CPA")
524+
self.m(hn.first, "John", hn)
525+
self.m(hn.last, "Doe", hn)
526+
self.m(hn.suffix, "JD.CPA", hn)
527+
528+
def test_joined_conjunctions_do_not_leak_into_conjunctions(self) -> None:
529+
hn = self._assert_parse_leaves_config_unchanged("Louis of the Netherlands")
530+
self.m(hn.first, "Louis of the Netherlands", hn)
531+
532+
def test_title_conjunction_join_does_not_leak_into_titles(self) -> None:
533+
hn = self._assert_parse_leaves_config_unchanged("Mr. and Mrs. John Smith")
534+
self.m(hn.title, "Mr. and Mrs.", hn)
535+
self.m(hn.first, "John", hn)
536+
self.m(hn.last, "Smith", hn)
537+
538+
def test_prefix_conjunction_join_does_not_leak_into_prefixes(self) -> None:
539+
hn = self._assert_parse_leaves_config_unchanged("Alois von und zu Liechtenstein")
540+
self.m(hn.first, "Alois", hn)
541+
self.m(hn.last, "von und zu Liechtenstein", hn)
542+
543+
def test_instance_owned_constants_not_mutated_by_parsing(self) -> None:
544+
hn = HumanName("", constants=None)
545+
before = {a: set(getattr(hn.C, a)) for a in self._WATCHED_ATTRS}
546+
hn.full_name = "Lt.Gov. John Doe"
547+
for attr, snapshot in before.items():
548+
leaked = set(getattr(hn.C, attr)) - snapshot
549+
self.assertEqual(leaked, set(),
550+
f"parsing leaked {leaked!r} into the instance's own C.{attr}")
551+
self.m(hn.title, "Lt.Gov.", hn)
552+
553+
def test_derivations_reset_between_parses_of_same_instance(self) -> None:
554+
# Re-assigning full_name re-parses; each parse must re-derive from a
555+
# clean slate and still resolve the period-joined title.
556+
hn = HumanName("Lt.Gov. John Doe")
557+
hn.full_name = "Lt.Gov. Jane Roe"
558+
self.m(hn.title, "Lt.Gov.", hn)
559+
self.m(hn.first, "Jane", hn)
560+
self.m(hn.last, "Roe", hn)
561+
562+
490563
class SuffixesPrefixesTitlesPerformanceTests(HumanNameTestBase):
491564
"""Guard against accidental cache removal on suffixes_prefixes_titles.
492565

0 commit comments

Comments
 (0)