From 155fb733c088307ac9a0f6834da398e8f3ce1263 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 13:44:02 -0700 Subject: [PATCH 1/7] Unify i==0/else branches in join_on_conjunctions conjunction loop --- nameparser/parser.py | 24 +++++++----------------- tests/test_conjunctions.py | 13 +++++++++++++ 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/nameparser/parser.py b/nameparser/parser.py index d4d9cd6..1a1487e 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -1295,23 +1295,13 @@ def shift_conj_index(past: int, by: int) -> None: # http://code.google.com/p/python-nameparser/issues/detail?id=11 continue - if i == 0: - new_piece = " ".join(pieces[i:i+2]) - register_joined_piece(new_piece, pieces[i+1]) - pieces[i] = new_piece - pieces.pop(i+1) - shift_conj_index(past=i, by=1) - - else: - new_piece = " ".join(pieces[i-1:i+2]) - register_joined_piece(new_piece, pieces[i-1]) - pieces[i-1] = new_piece - # len(pieces) - i is always >= 1 here: pieces[i-1:i+2] above - # already accessed index i, so i is guaranteed in range. - rm_count = min(2, len(pieces) - i) - assert rm_count > 0, f"unexpected empty deletion at i={i}, pieces={pieces}" - del pieces[i:i+rm_count] - shift_conj_index(past=i, by=rm_count) + start = max(0, i - 1) + end = min(len(pieces), i + 2) + new_piece = " ".join(pieces[start:end]) + neighbor = pieces[start] if start < i else pieces[end - 1] + register_joined_piece(new_piece, neighbor) + pieces[start:end] = [new_piece] + shift_conj_index(past=i, by=end - start - 1) # join prefixes to following lastnames: ['de la Vega'], ['van Buren'] prefixes = list(filter(self.is_prefix, pieces)) diff --git a/tests/test_conjunctions.py b/tests/test_conjunctions.py index 05b27de..286d8a6 100644 --- a/tests/test_conjunctions.py +++ b/tests/test_conjunctions.py @@ -274,3 +274,16 @@ def test_conjunction_bridges_prefix_chain_with_multiple_conjunctions(self) -> No self.m(hn.first, "Alois", hn) self.m(hn.middle, "", hn) self.m(hn.last, "von und zu und von Liechtenstein", hn) + + def test_leading_conjunction_joins_to_following_piece(self) -> None: + # exercises the i == 0 branch: conjunction is the very first piece + hn = HumanName("and Jon Dough") + self.m(hn.first, "and Jon", hn) + self.m(hn.last, "Dough", hn) + + def test_trailing_conjunction_at_list_end(self) -> None: + # exercises the truncated rm_count path: conjunction is the last piece, + # so there is no following piece to remove + hn = HumanName("Jon Dough and") + self.m(hn.first, "Jon", hn) + self.m(hn.last, "Dough and", hn) From cc01385cafd64aa1b67c6a50f188df6d95abef6a Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 13:46:52 -0700 Subject: [PATCH 2/7] Remove duplicate conjunction tests added in 155fb73 test_leading_conjunction_joins_to_following_piece and test_trailing_conjunction_at_list_end were exact duplicates of the pre-existing test_starts_with_conjunction and test_ends_with_conjunction tests (same input, same assertions). They added no new coverage of the join_on_conjunctions restructuring and only inflated the test file. Co-Authored-By: Claude Sonnet 5 --- tests/test_conjunctions.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/test_conjunctions.py b/tests/test_conjunctions.py index 286d8a6..05b27de 100644 --- a/tests/test_conjunctions.py +++ b/tests/test_conjunctions.py @@ -274,16 +274,3 @@ def test_conjunction_bridges_prefix_chain_with_multiple_conjunctions(self) -> No self.m(hn.first, "Alois", hn) self.m(hn.middle, "", hn) self.m(hn.last, "von und zu und von Liechtenstein", hn) - - def test_leading_conjunction_joins_to_following_piece(self) -> None: - # exercises the i == 0 branch: conjunction is the very first piece - hn = HumanName("and Jon Dough") - self.m(hn.first, "and Jon", hn) - self.m(hn.last, "Dough", hn) - - def test_trailing_conjunction_at_list_end(self) -> None: - # exercises the truncated rm_count path: conjunction is the last piece, - # so there is no following piece to remove - hn = HumanName("Jon Dough and") - self.m(hn.first, "Jon", hn) - self.m(hn.last, "Dough and", hn) From 090832580812c7792ba1d4e5bb4d7aacb2ee08a7 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 13:49:40 -0700 Subject: [PATCH 3/7] Fix repeated-prefix-chain data loss in join_on_conjunctions prefix loop Replace value-based pieces.index() lookup with a position-tracked forward scan. The old code re-found each prefix by searching for its string value after the list had already been mutated, which silently dropped one repetition of a chain when the same prefix word appeared more than once (e.g. 'Juan de la de la Vega' -> last 'de la Vega', missing the first 'de la'). Fixes #208. --- nameparser/parser.py | 59 ++++++++++++------------------------------ tests/test_prefixes.py | 10 +++++++ 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/nameparser/parser.py b/nameparser/parser.py index 1a1487e..ad9918b 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -1304,51 +1304,26 @@ def shift_conj_index(past: int, by: int) -> None: shift_conj_index(past=i, by=end - start - 1) # join prefixes to following lastnames: ['de la Vega'], ['van Buren'] - prefixes = list(filter(self.is_prefix, pieces)) - if prefixes: - for prefix in prefixes: - try: - i = pieces.index(prefix) - except ValueError: - # If the prefix is no longer in pieces, it's because it has been - # combined with the prefix that appears right before (or before that when - # chained together) in the last loop, so the index of that newly created - # piece is the same as in the last loop, i==i still, and we want to join - # it to the next piece. - pass + i = 0 + while i < len(pieces): + if not self.is_prefix(pieces[i]) or (i == 0 and total_length >= 1): + # If it's the first piece and there are more than 1 rootnames, + # assume it's a first name rather than a prefix. + i += 1 + continue - new_piece = '' + # absorb any immediately-adjacent prefixes into one contiguous run + # e.g. "von und zu der" ==> chain them all before looking further + j = i + 1 + while j < len(pieces) and self.is_prefix(pieces[j]): + j += 1 - # join everything after the prefix until the next prefix or suffix + # then join everything after the run until the next prefix or suffix + while j < len(pieces) and not self.is_prefix(pieces[j]) and not self.is_suffix(pieces[j]): + j += 1 - try: - if i == 0 and total_length >= 1: - # If it's the first piece and there are more than 1 rootnames, assume it's a first name - continue - next_prefix = next(iter(filter(self.is_prefix, pieces[i + 1:]))) - j = pieces.index(next_prefix, i + 1) - if j == i + 1: - # if there are two prefixes in sequence, join to the following piece - j += 1 - new_piece = ' '.join(pieces[i:j]) - pieces[i:j] = [new_piece] - except StopIteration: - try: - # if there are no more prefixes, look for a suffix to stop at - stop_at = next(iter(filter(self.is_suffix, pieces[i + 1:]))) - # search from i + 1: filter() finds the value of stop_at - # in pieces[i+1:] but pieces.index() without a start - # argument searches from 0, so an earlier occurrence of - # the same token (e.g. a suffix token that also appears - # before the prefix) would be matched instead. - j = pieces.index(stop_at, i + 1) - new_piece = ' '.join(pieces[i:j]) - pieces[i:j] = [new_piece] - except StopIteration: - # if there were no suffixes, nothing to stop at so join all - # remaining pieces - new_piece = ' '.join(pieces[i:]) - pieces[i:] = [new_piece] + pieces[i:j] = [' '.join(pieces[i:j])] + i += 1 log.debug("pieces: %s", pieces) return pieces diff --git a/tests/test_prefixes.py b/tests/test_prefixes.py index 216ad49..fa8643c 100644 --- a/tests/test_prefixes.py +++ b/tests/test_prefixes.py @@ -307,6 +307,16 @@ def test_non_first_name_prefix_with_custom_title(self) -> None: self.m(hn.first, "", hn) self.m(hn.last, "de Mesnil", hn) + def test_repeated_prefix_chain_de_la(self) -> None: + hn = HumanName("Juan de la de la Vega") + self.m(hn.first, "Juan", hn) + self.m(hn.last, "de la de la Vega", hn) + + def test_repeated_prefix_chain_van_der(self) -> None: + hn = HumanName("Charles van der van der Berg") + self.m(hn.first, "Charles", hn) + self.m(hn.last, "van der van der Berg", hn) + # --- safety: excluded / ambiguous particles are unchanged --- def test_leading_von_is_unchanged(self) -> None: From 6a7532d351823d53d9e0c965b5aa52fdcb645500 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 13:54:07 -0700 Subject: [PATCH 4/7] Fix stale comment on leading-prefix skip condition Code review noted the comment said "more than 1 rootnames" but the actual condition is total_length >= 1, which is true for essentially all real input. Clarify wording to match the condition. --- nameparser/parser.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nameparser/parser.py b/nameparser/parser.py index ad9918b..2e1ca1c 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -1307,8 +1307,9 @@ def shift_conj_index(past: int, by: int) -> None: i = 0 while i < len(pieces): if not self.is_prefix(pieces[i]) or (i == 0 and total_length >= 1): - # If it's the first piece and there are more than 1 rootnames, - # assume it's a first name rather than a prefix. + # If it's the first piece and there's at least 1 rootname + # elsewhere, assume this piece is a first name rather than a + # prefix (total_length >= 1 covers essentially all real input). i += 1 continue From a5aff9486e563ae01c4fe70d1eacfe99c669691f Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 14:11:14 -0700 Subject: [PATCH 5/7] Add regression tests for triple-chained prefixes and chain+suffix PR review flagged that the current tests only cover the narrowest case (a single prefix phrase repeated twice). Add two stronger guards for the exact code path #208's fix touches: 3+ chained repeats, and a repeated chain immediately followed by a suffix (the prefix-run absorption loop and the suffix-boundary loop share the same index variable, so this combination is worth pinning down explicitly). --- tests/test_prefixes.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_prefixes.py b/tests/test_prefixes.py index fa8643c..46842ce 100644 --- a/tests/test_prefixes.py +++ b/tests/test_prefixes.py @@ -317,6 +317,23 @@ def test_repeated_prefix_chain_van_der(self) -> None: self.m(hn.first, "Charles", hn) self.m(hn.last, "van der van der Berg", hn) + def test_triple_repeated_prefix_chain(self) -> None: + # a stronger regression guard than the 2-repeat cases above: the + # contiguous-prefix absorption loop should chain any number of + # repeats, not just handle exactly two + hn = HumanName("Juan de la de la de la Vega") + self.m(hn.first, "Juan", hn) + self.m(hn.last, "de la de la de la Vega", hn) + + def test_repeated_prefix_chain_followed_by_suffix(self) -> None: + # the prefix-run absorption loop and the suffix-boundary loop share + # the same index variable, so a repeated chain immediately + # followed by a suffix is worth pinning down explicitly + hn = HumanName("Juan de la de la Vega Jr.") + self.m(hn.first, "Juan", hn) + self.m(hn.last, "de la de la Vega", hn) + self.m(hn.suffix, "Jr.", hn) + # --- safety: excluded / ambiguous particles are unchanged --- def test_leading_von_is_unchanged(self) -> None: From 1bfbbe4d579bfa8163bd15197088ce8eca731fe2 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 14:22:51 -0700 Subject: [PATCH 6/7] Extract leading_first_name from bundled guard condition Code review noted the compound condition conflated "is this piece a prefix candidate at all" with "is it exempted as a leading first name" into one boolean expression. Name the second clause so its rationale is legible at the call site. --- nameparser/parser.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nameparser/parser.py b/nameparser/parser.py index 2e1ca1c..f970a02 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -1306,10 +1306,10 @@ def shift_conj_index(past: int, by: int) -> None: # join prefixes to following lastnames: ['de la Vega'], ['van Buren'] i = 0 while i < len(pieces): - if not self.is_prefix(pieces[i]) or (i == 0 and total_length >= 1): - # If it's the first piece and there's at least 1 rootname - # elsewhere, assume this piece is a first name rather than a - # prefix (total_length >= 1 covers essentially all real input). + # total_length >= 1 covers essentially all real input, so this + # treats any leading piece as a first name rather than a prefix. + leading_first_name = i == 0 and total_length >= 1 + if not self.is_prefix(pieces[i]) or leading_first_name: i += 1 continue From 947b1aa6f3d43b3492eef9e58959c85d1ba75489 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 14:29:20 -0700 Subject: [PATCH 7/7] Simplify contiguous-conjunction join to a single reverse-order loop group_contiguous_integers already returns non-overlapping ranges, so there's no need to flatten each range into individual indices and delete them one at a time. Process the ranges themselves in reverse and slice-assign each one directly, matching the same pattern already used by the two other loops in this method. --- nameparser/parser.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/nameparser/parser.py b/nameparser/parser.py index f970a02..f7f6d2f 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -1251,18 +1251,14 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = contiguous_conj_i = group_contiguous_integers(conj_index) - delete_i: list[int] = [] - for cont_i in contiguous_conj_i: + # process ranges in reverse so deleting one range doesn't shift the + # indices of ranges still to be processed + for cont_i in reversed(contiguous_conj_i): new_piece = " ".join(pieces[cont_i[0]: cont_i[1]+1]) - delete_i += list(range(cont_i[0]+1, cont_i[1]+1)) - pieces[cont_i[0]] = new_piece + pieces[cont_i[0]:cont_i[1]+1] = [new_piece] # add newly joined conjunctions to constants to be found later self.C.conjunctions.add(new_piece) - for i in reversed(delete_i): - # delete pieces in reverse order or the index changes on each delete - del pieces[i] - if len(pieces) == 1: # if there's only one piece left, nothing left to do return pieces