Skip to content

Commit ae9bcc9

Browse files
derek73claude
andcommitted
Address review: guard suffix-comma path, pin setter filtering, docs
- Add the missing `if part` guard to the suffix-comma parts[1:] loop: "John Doe, Jr.,," leaked '' into suffix_list via expand_suffix_delimiter('') returning [''], the same bug class this branch fixes elsewhere (regression test added) - Pin the setter-path filtering as intended behavior: whitespace-only and empty tokens assigned via attributes, lists, or __setitem__ no longer become *_list members - Pin the lastname_pieces call site: ", John" leaves last_list empty - Assert sibling lists for HumanName(',') so a relocated (not removed) empty piece can't pass - State the no-empty-pieces guarantee in parse_pieces()'s docstring and broaden the release log entry to cover last_list/suffix_list and the setter behavior - Align the suffix-comma "final piece" comment with the no-comma site's wording (the PR 216 replace_all missed this deeper-indented copy) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 38c4944 commit ae9bcc9

4 files changed

Lines changed: 45 additions & 6 deletions

File tree

docs/release_log.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Release Log
77
- Fix a trailing suffix being silently dropped after an empty comma segment, e.g. ``"Doe, John,, Jr."`` losing the ``"Jr."``
88
- Remove ``__ne__``; Python 3 derives ``!=`` from ``__eq__`` automatically
99
- 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
10-
- Fix degenerate comma-only input (e.g. ``","``, ``"Doe,, Jr."``) leaving an empty-string member in ``first_list``
10+
- Fix degenerate comma input (a bare ``","`` or an empty comma segment, e.g. ``"Doe,, Jr."``, ``"John Doe, Jr.,,"``) leaving an empty-string member in ``first_list``, ``last_list``, or ``suffix_list``; whitespace-only tokens assigned via the setters are dropped the same way
1111
- 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)
1212
- 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)
1313
- 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: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,10 @@ def parse_full_name(self) -> None:
10811081
# parts[0], parts[1:...]
10821082

10831083
for part in parts[1:]:
1084-
self.suffix_list += self.expand_suffix_delimiter(part)
1084+
# skip empty segments from doubled commas, mirroring the
1085+
# parts[2:] guard in the lastname-comma path below
1086+
if part:
1087+
self.suffix_list += self.expand_suffix_delimiter(part)
10851088
pieces = self.parse_pieces(parts[0].split(' '))
10861089
pieces = self._join_bound_first_name(pieces, reserve_last=True)
10871090
log.debug("pieces: %s", str(pieces))
@@ -1100,9 +1103,10 @@ def parse_full_name(self) -> None:
11001103
self.first_list.append(piece)
11011104
continue
11021105
if self.are_suffixes(pieces[i+1:]):
1103-
# the final piece always lands here: are_suffixes() is
1104-
# vacuously True for the empty tail, making this the
1105-
# last-name branch as well as the suffix branch
1106+
# any piece reaching this check as the final piece
1107+
# lands here: are_suffixes() is vacuously True for the
1108+
# empty tail, making this the last-name branch as well
1109+
# as the suffix branch
11061110
self.last_list.append(piece)
11071111
self.suffix_list = pieces[i+1:] + self.suffix_list
11081112
break
@@ -1164,7 +1168,9 @@ def parse_full_name(self) -> None:
11641168
def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) -> list[str]:
11651169
"""
11661170
Split parts on spaces and remove commas, join on conjunctions and
1167-
lastname prefixes. If parts have periods in the middle, try splitting
1171+
lastname prefixes. Tokens that are empty after stripping spaces and
1172+
commas are dropped, so the returned pieces never contain empty
1173+
strings. If parts have periods in the middle, try splitting
11681174
on periods and check if the parts are titles or suffixes. If they are
11691175
add to the constant so they will be found.
11701176

tests/test_python_api.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,34 @@ def test_degenerate_comma_input_leaves_no_empty_pieces(self) -> None:
300300
# first_list — a silent empty member in the public *_list attributes.
301301
hn = HumanName(",")
302302
self.assertEqual(hn.first_list, [])
303+
self.assertEqual(hn.middle_list, [])
304+
self.assertEqual(hn.last_list, [])
303305
self.assertEqual(len(hn), 0)
304306
hn = HumanName("Doe,, Jr.")
305307
self.assertEqual(hn.first_list, [])
306308
self.m(hn.last, "Doe", hn)
307309
self.m(hn.suffix, "Jr.", hn)
310+
# empty parts[0] exercises the lastname_pieces call site: the empty
311+
# last-name segment must not become a member of last_list
312+
hn = HumanName(", John")
313+
self.assertEqual(hn.last_list, [])
314+
self.m(hn.first, "John", hn)
315+
316+
def test_assignment_filters_empty_tokens(self) -> None:
317+
# parse_pieces() drops tokens that strip to nothing at every entry
318+
# point, including the setters: whitespace-only strings and empty
319+
# list members never become *_list members (they previously survived,
320+
# e.g. hn.first = ' ' left first == ' ' and middle 'a b' gained an
321+
# empty member from ['a', '', 'b']).
322+
hn = HumanName("John Doe")
323+
hn.first = " "
324+
self.assertEqual(hn.first_list, [])
325+
self.m(hn.first, "", hn)
326+
hn.middle = ["a", "", "b"]
327+
self.assertEqual(hn.middle_list, ["a", "b"])
328+
self.m(hn.middle, "a b", hn)
329+
hn["last"] = ["", "Smith"]
330+
self.assertEqual(hn.last_list, ["Smith"])
308331

309332
def test_blank_name(self) -> None:
310333
hn = HumanName()

tests/test_suffixes.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,3 +436,13 @@ def test_empty_comma_segment_does_not_drop_following_suffix(self) -> None:
436436
# regression that the single-empty case above would not catch)
437437
hn = HumanName("Doe, John,, Jr.,, III")
438438
self.m(hn.suffix, "Jr., III", hn)
439+
440+
def test_suffix_comma_empty_segment_not_added_to_suffix_list(self) -> None:
441+
# Regression: in suffix-comma format ("John Doe, Jr.,," -- one
442+
# trailing comma survives collapse_whitespace), the unguarded
443+
# parts[1:] loop leaked '' into suffix_list via
444+
# expand_suffix_delimiter('') returning [''].
445+
hn = HumanName("John Doe, Jr.,,")
446+
self.assertEqual(hn.suffix_list, ["Jr."])
447+
self.m(hn.first, "John", hn)
448+
self.m(hn.last, "Doe", hn)

0 commit comments

Comments
 (0)