Skip to content

Commit 546331f

Browse files
derek73claude
andcommitted
Fix bugs hidden in untested parser paths; remove vestigial unparsable
Auditing the 11 uncovered lines in parser.py (98% coverage) turned up three real bugs, dead code, and a few untested-but-live paths: - Fix __hash__ to lowercase like __eq__ does, so equal HumanName instances hash equal and behave correctly in sets and dicts - Fix initials() emitting a stray empty initial ("J. . V.") — or raising TypeError when empty_attribute_default is None — for name parts with no initialable words (e.g. prefix-only middle "de la"); deduplicate the per-group comprehensions into _initials_lists() - Fix a trailing suffix being silently dropped after an empty comma segment, e.g. "Doe, John,, Jr." losing the "Jr." - Remove the unparsable attribute: the len(self) < 0 guard meant to set it was unreachable, so it has reported False for every parsed name since the earliest releases - Remove __ne__ (Python 3 derives != from __eq__) and the two unreachable "if not nxt" branches in the parse loops, which the vacuously-true are_suffixes() check always preempts - Rename __process_initial__ to _process_initial: dunder-both-sides names are reserved for Python special methods Adds dunder-contract, initials, and suffix regression tests; parser.py is now at 100% statement coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 71a889e commit 546331f

7 files changed

Lines changed: 107 additions & 59 deletions

File tree

docs/release_log.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
Release Log
22
===========
33
* 1.3.0 - Unreleased
4+
- Remove the vestigial ``unparsable`` attribute: the guard that was meant to set it was unreachable, so it has reported ``False`` for every parsed name since the earliest releases
5+
- Fix ``__hash__`` to lowercase the name like ``__eq__`` does, so equal ``HumanName`` instances hash equal and behave correctly in sets and dicts
6+
- 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"``
7+
- Fix a trailing suffix being silently dropped after an empty comma segment, e.g. ``"Doe, John,, Jr."`` losing the ``"Jr."``
8+
- Remove ``__ne__``; Python 3 derives ``!=`` from ``__eq__`` automatically
9+
- Change internal initials helper ``__process_initial__`` to ``_process_initial``: double-underscore-both-sides names are reserved for Python special methods; subclasses overriding the old name must rename their override
410
- Add ``non_first_name_prefixes`` to ``Constants``: a leading particle that is never a first name (e.g. ``"de Mesnil"``, ``"dos Santos"``) now parses as a surname with an empty first name, instead of treating the particle as the first name (closes #121)
511
- Add a first-class ``maiden`` field and ``maiden_delimiters`` to ``Constants``, so a delimiter (e.g. parenthesis) can be routed to ``maiden`` instead of ``nickname`` for alternate/maiden surnames, e.g. ``"Baker (Johnson), Jenny"`` (closes #22)
612
- Fix suffix-shaped parenthesized/quoted content (e.g. ``"(Ret)"``, ``"(MBA)"``) being misclassified as a nickname instead of a suffix (closes #111)

nameparser/parser.py

Lines changed: 42 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,6 @@ class HumanName:
8282

8383
_count = 0
8484
_members = ['title', 'first', 'middle', 'last', 'suffix', 'nickname', 'maiden']
85-
unparsable = True
8685
_full_name = ''
8786

8887
title_list: list[str]
@@ -131,7 +130,6 @@ def __init__(
131130
self.suffix = suffix
132131
self.nickname = nickname
133132
self.maiden = maiden
134-
self.unparsable = False
135133
else:
136134
# full_name setter triggers the parse
137135
self.full_name = full_name
@@ -163,9 +161,6 @@ def __eq__(self, other: object) -> bool:
163161
"""
164162
return str(self).lower() == str(other).lower()
165163

166-
def __ne__(self, other: object) -> bool:
167-
return not str(self).lower() == str(other).lower()
168-
169164
@overload
170165
def __getitem__(self, key: slice) -> list[str]: ...
171166
@overload
@@ -202,23 +197,21 @@ def __str__(self) -> str:
202197
return " ".join(self)
203198

204199
def __hash__(self) -> int:
205-
return hash(str(self))
200+
# __eq__ compares lowercased strings, so hash the lowercased string
201+
# to keep equal instances in the same hash bucket.
202+
return hash(str(self).lower())
206203

207204
def __repr__(self) -> str:
208-
if self.unparsable:
209-
_string = f"<{self.__class__.__name__} : [ Unparsable ] >"
210-
else:
211-
attrs = (
212-
f" title: {self.title or ''!r}\n"
213-
f" first: {self.first or ''!r}\n"
214-
f" middle: {self.middle or ''!r}\n"
215-
f" last: {self.last or ''!r}\n"
216-
f" suffix: {self.suffix or ''!r}\n"
217-
f" nickname: {self.nickname or ''!r}\n"
218-
f" maiden: {self.maiden or ''!r}"
219-
)
220-
_string = f"<{self.__class__.__name__} : [\n{attrs}\n]>"
221-
return _string
205+
attrs = (
206+
f" title: {self.title or ''!r}\n"
207+
f" first: {self.first or ''!r}\n"
208+
f" middle: {self.middle or ''!r}\n"
209+
f" last: {self.last or ''!r}\n"
210+
f" suffix: {self.suffix or ''!r}\n"
211+
f" nickname: {self.nickname or ''!r}\n"
212+
f" maiden: {self.maiden or ''!r}"
213+
)
214+
return f"<{self.__class__.__name__} : [\n{attrs}\n]>"
222215

223216
def as_dict(self, include_empty: bool = True) -> dict[str, str]:
224217
"""
@@ -246,7 +239,7 @@ def as_dict(self, include_empty: bool = True) -> dict[str, str]:
246239
d[m] = val
247240
return d
248241

249-
def __process_initial__(self, name_part: str, firstname: bool = False) -> str:
242+
def _process_initial(self, name_part: str, firstname: bool = False) -> str:
250243
"""
251244
Name parts may include prefixes or conjunctions. This function filters these from the name unless it is
252245
a first name, since first names cannot be conjunctions or prefixes.
@@ -259,8 +252,21 @@ def __process_initial__(self, name_part: str, firstname: bool = False) -> str:
259252
initials.append(part[0])
260253
if len(initials) > 0:
261254
return self.initials_separator.join(initials)
262-
else:
263-
return self.C.empty_attribute_default
255+
# Return '' (never empty_attribute_default, which may be None) when a
256+
# part has no initialable words, e.g. a middle name consisting only of
257+
# prefixes ("de la"). Callers drop these parts entirely.
258+
return ''
259+
260+
def _initials_lists(self) -> tuple[list[str], list[str], list[str]]:
261+
"""Initials for the first, middle and last name groups. Parts that
262+
yield no initials (e.g. a prefix-only middle name like "de la") are
263+
dropped rather than kept as empty strings.
264+
"""
265+
def group_initials(names: list[str], firstname: bool = False) -> list[str]:
266+
return [i for i in (self._process_initial(n, firstname) for n in names if n) if i]
267+
return (group_initials(self.first_list, True),
268+
group_initials(self.middle_list),
269+
group_initials(self.last_list))
264270

265271
def initials_list(self) -> list[str]:
266272
"""
@@ -275,9 +281,7 @@ def initials_list(self) -> list[str]:
275281
>>> name.initials_list()
276282
['J', 'D']
277283
"""
278-
first_initials_list = [self.__process_initial__(name, True) for name in self.first_list if name]
279-
middle_initials_list = [self.__process_initial__(name) for name in self.middle_list if name]
280-
last_initials_list = [self.__process_initial__(name) for name in self.last_list if name]
284+
first_initials_list, middle_initials_list, last_initials_list = self._initials_lists()
281285
return first_initials_list + middle_initials_list + last_initials_list
282286

283287
def initials(self) -> str:
@@ -303,9 +307,7 @@ def initials(self) -> str:
303307
'J A D'
304308
"""
305309

306-
first_initials_list = [self.__process_initial__(name, True) for name in self.first_list if name]
307-
middle_initials_list = [self.__process_initial__(name) for name in self.middle_list if name]
308-
last_initials_list = [self.__process_initial__(name) for name in self.last_list if name]
310+
first_initials_list, middle_initials_list, last_initials_list = self._initials_lists()
309311

310312
# Empty parts must render as '' (not empty_attribute_default, which may be
311313
# None) so str.format does not interpolate the literal "None" into the
@@ -996,7 +998,6 @@ def parse_full_name(self) -> None:
996998
self.suffix_list = []
997999
self.nickname_list = []
9981000
self.maiden_list = []
999-
self.unparsable = True
10001001

10011002
self.pre_process()
10021003

@@ -1043,12 +1044,12 @@ def parse_full_name(self) -> None:
10431044
self.is_roman_numeral(nxt) and i == p_len - 2
10441045
and not self.is_an_initial(piece)
10451046
):
1047+
# the final piece always lands here: are_suffixes() is
1048+
# vacuously True for the empty tail, making this the
1049+
# last-name branch as well as the suffix branch
10461050
self.last_list.append(piece)
10471051
self.suffix_list += pieces[i+1:]
10481052
break
1049-
if not nxt:
1050-
self.last_list.append(piece)
1051-
continue
10521053

10531054
self.middle_list.append(piece)
10541055
else:
@@ -1092,12 +1093,12 @@ def parse_full_name(self) -> None:
10921093
self.first_list.append(piece)
10931094
continue
10941095
if self.are_suffixes(pieces[i+1:]):
1096+
# the final piece always lands here: are_suffixes() is
1097+
# vacuously True for the empty tail, making this the
1098+
# last-name branch as well as the suffix branch
10951099
self.last_list.append(piece)
10961100
self.suffix_list = pieces[i+1:] + self.suffix_list
10971101
break
1098-
if not nxt:
1099-
self.last_list.append(piece)
1100-
continue
11011102
self.middle_list.append(piece)
11021103
else:
11031104

@@ -1145,17 +1146,12 @@ def parse_full_name(self) -> None:
11451146
self.suffix_list.append(piece)
11461147
continue
11471148
self.middle_list.append(piece)
1148-
try:
1149-
if parts[2]:
1150-
for part in parts[2:]:
1151-
self.suffix_list += self.expand_suffix_delimiter(part)
1152-
except IndexError:
1153-
pass
1149+
for part in parts[2:]:
1150+
# skip empty segments from doubled commas ("Doe, John,, Jr.")
1151+
# without dropping the segments that follow them
1152+
if part:
1153+
self.suffix_list += self.expand_suffix_delimiter(part)
11541154

1155-
if len(self) < 0:
1156-
log.info("Unparsable: \"%s\" ", self.original)
1157-
else:
1158-
self.unparsable = False
11591155
self.post_process()
11601156

11611157
def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) -> list[str]:

tests/test_initials.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ def test_initials_all_empty_returns_empty_attribute_default(self) -> None:
3737
hn.C.empty_attribute_default = None
3838
self.assertEqual(hn.initials(), None)
3939

40+
def test_initials_middle_name_all_prefixes(self) -> None:
41+
# "Vega, Juan de la" parses with middle name "de la", which contains
42+
# no initialable words (both are prefixes). The part must be skipped
43+
# entirely — not emit an empty initial ("J. . V.") and not crash when
44+
# empty_attribute_default is None.
45+
hn = HumanName("Vega, Juan de la")
46+
self.m(hn.middle, "de la", hn)
47+
self.assertEqual(hn.initials_list(), ["J", "V"])
48+
self.assertEqual(hn.initials(), "J. V.")
49+
4050
def test_initials_complex_name(self) -> None:
4151
hn = HumanName("Doe, John A. Kenneth, Jr.")
4252
self.m(hn.initials(), "J. A. K. D.", hn)
@@ -111,11 +121,11 @@ def test_initials_separator_kwarg(self) -> None:
111121
self.m(hn.initials(), "J.A.K.D.", hn)
112122

113123
def test_initials_separator_custom_value(self) -> None:
114-
# Non-empty custom separator exercising __process_initial__ on a multi-word
124+
# Non-empty custom separator exercising _process_initial on a multi-word
115125
# token. "Van Berg" is a single name part whose two words produce two initials
116126
# joined by initials_separator.
117127
hn = HumanName("", initials_separator="-", initials_delimiter=".")
118-
result = hn.__process_initial__("Van Berg", firstname=True)
128+
result = hn._process_initial("Van Berg", firstname=True)
119129
self.assertEqual(result, "V-B")
120130

121131
def test_str_default_behavior_unchanged(self) -> None:
@@ -126,46 +136,39 @@ def test_str_default_behavior_unchanged(self) -> None:
126136

127137
def test_constructor_first(self) -> None:
128138
hn = HumanName(first="TheName")
129-
self.assertFalse(hn.unparsable)
130139
self.m(hn.first, "TheName", hn)
131140

132141
def test_constructor_middle(self) -> None:
133142
hn = HumanName(middle="TheName")
134-
self.assertFalse(hn.unparsable)
135143
self.m(hn.middle, "TheName", hn)
136144

137145
def test_constructor_last(self) -> None:
138146
hn = HumanName(last="TheName")
139-
self.assertFalse(hn.unparsable)
140147
self.m(hn.last, "TheName", hn)
141148

142149
def test_constructor_title(self) -> None:
143150
hn = HumanName(title="TheName")
144-
self.assertFalse(hn.unparsable)
145151
self.m(hn.title, "TheName", hn)
146152

147153
def test_constructor_suffix(self) -> None:
148154
hn = HumanName(suffix="TheName")
149-
self.assertFalse(hn.unparsable)
150155
self.m(hn.suffix, "TheName", hn)
151156

152157
def test_constructor_nickname(self) -> None:
153158
hn = HumanName(nickname="TheName")
154-
self.assertFalse(hn.unparsable)
155159
self.m(hn.nickname, "TheName", hn)
156160

157161
def test_constructor_multiple(self) -> None:
158162
hn = HumanName(first="TheName", last="lastname", title="mytitle", full_name="donotparse")
159-
self.assertFalse(hn.unparsable)
160163
self.m(hn.first, "TheName", hn)
161164
self.m(hn.last, "lastname", hn)
162165
self.m(hn.title, "mytitle", hn)
163166

164167
def test_initials_separator_kwarg_multiword_part(self) -> None:
165-
# Regression: initials_separator kwarg must flow into __process_initial__
168+
# Regression: initials_separator kwarg must flow into _process_initial
166169
# for multi-word name parts, not just into the initials() join calls.
167170
hn = HumanName("", initials_separator="")
168-
result = hn.__process_initial__("Van Berg", firstname=True)
171+
result = hn._process_initial("Van Berg", firstname=True)
169172
self.assertEqual(result, "VB")
170173

171174
def test_string_format_empty_string_kwarg(self) -> None:

tests/test_nicknames.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ def test_maiden_via_constructor_kwarg(self) -> None:
232232
self.m(hn.first, "Jenny", hn)
233233
self.m(hn.last, "Baker", hn)
234234
self.m(hn.maiden, "Johnson", hn)
235-
self.assertFalse(hn.unparsable)
236235

237236
def test_maiden_name_in_parenthesis_with_comma(self) -> None:
238237
C = Constants()

tests/test_prefixes.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ def test_many_repeated_prefixes_does_not_blow_up(self) -> None:
8787
# regression has been reintroduced.
8888
name = "Jan " + "van der " * 30 + "Berg"
8989
hn = HumanName(name)
90-
self.assertFalse(hn.unparsable)
9190
self.m(hn.first, "Jan", hn)
9291
self.assertIn("Berg", hn.last)
9392

tests/test_python_api.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,36 @@ def test_comparison_case_insensitive(self) -> None:
215215
self.assertIsNot(hn1, hn2)
216216
self.assertTrue(hn1 == "Dr. John P. Doe-ray clu, CFP, LUTC")
217217

218+
def test_hash_matches_case_insensitive_equality(self) -> None:
219+
# __eq__ compares lowercased strings, so __hash__ must too:
220+
# equal objects are required to have equal hashes.
221+
hn1 = HumanName("Doe-Ray, Dr. John P., CLU, CFP, LUTC")
222+
hn2 = HumanName("dr. john p. doe-Ray, CLU, CFP, LUTC")
223+
self.assertEqual(hn1, hn2)
224+
self.assertEqual(hash(hn1), hash(hn2))
225+
self.assertEqual(len({hn1, hn2}), 1)
226+
227+
def test_not_equal_operator(self) -> None:
228+
self.assertTrue(HumanName("John Smith") != HumanName("Jane Smith"))
229+
self.assertFalse(HumanName("John Smith") != HumanName("john smith"))
230+
231+
def test_unparsable_attribute_removed(self) -> None:
232+
# Removed in 1.3.0: the guard that reported unparsable names was
233+
# unreachable, so the attribute was always False after any parse.
234+
self.assertFalse(hasattr(HumanName("John Smith"), "unparsable"))
235+
self.assertFalse(hasattr(HumanName(first="John"), "unparsable"))
236+
237+
def test_str_fallback_without_string_format(self) -> None:
238+
# string_format=None falls back to joining the non-empty attributes
239+
hn = HumanName("Dr. John A. Doe, Jr.")
240+
hn.string_format = None
241+
self.assertEqual(str(hn), "Dr. John A. Doe Jr.")
242+
243+
def test_repr_blank_name(self) -> None:
244+
hn = HumanName()
245+
self.assertIn("first: ''", repr(hn))
246+
self.assertIn(hn.__class__.__name__, repr(hn))
247+
218248
def test_slice(self) -> None:
219249
hn = HumanName("Doe-Ray, Dr. John P., CLU, CFP, LUTC")
220250
self.m(list(hn), ['Dr.', 'John', 'P.', 'Doe-Ray', 'CLU, CFP, LUTC'], hn)
@@ -240,6 +270,11 @@ def test_setitem(self) -> None:
240270
with pytest.raises(TypeError):
241271
hn["suffix"] = {"test": "test"}
242272

273+
def test_setitem_invalid_key_raises_keyerror(self) -> None:
274+
hn = HumanName("Dr. John A. Kenneth Doe, Jr.")
275+
with pytest.raises(KeyError):
276+
hn["bogus"] = "value"
277+
243278
def test_conjunction_names(self) -> None:
244279
hn = HumanName("johnny y")
245280
self.m(hn.first, "johnny", hn)

tests/test_suffixes.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,3 +421,13 @@ def test_suffix_acronyms_ambiguous_removal_routes_to_suffix(self) -> None:
421421
hn = HumanName("Andrew Perkins (JD)", constants=C)
422422
self.m(hn.nickname, "", hn)
423423
self.m(hn.suffix, "JD", hn)
424+
425+
def test_empty_comma_segment_does_not_drop_following_suffix(self) -> None:
426+
# Regression: "Doe, John,, Jr." produced an empty third segment, and
427+
# the old `if parts[2]:` guard skipped every remaining segment --
428+
# silently dropping the trailing suffix. Empty segments should be
429+
# skipped individually.
430+
hn = HumanName("Doe, John,, Jr.")
431+
self.m(hn.first, "John", hn)
432+
self.m(hn.last, "Doe", hn)
433+
self.m(hn.suffix, "Jr.", hn)

0 commit comments

Comments
 (0)