Merge pull request #209 from derek73/fix/join-on-conjunctions-restructure

Fix repeated-prefix-chain data loss in join_on_conjunctions

Author: derek73

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
@@ -1295,70 +1291,36 @@ 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))
-        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):
+            # 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
 
-                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

tests/test_prefixes.py

@@ -307,6 +307,33 @@ 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)
+
+    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: