Skip to content

Commit 2f4e126

Browse files
derek73claude
andcommitted
review: improve comment accuracy and test coverage
- Fix inline comment in join_on_conjunctions: clarify that filter() finds the value in pieces[i+1:] but index() searches from 0 by default, and drop the misleading "title" framing (the token only needs to satisfy is_suffix, not is_title) - Add test for two-word prefix collision ("van der") — different loop iteration count than the single-word case - Add test with a genuine middle name alongside the repeated token, since the pre-fix bug corrupted the middle field specifically - Add @pytest.mark.timeout(2) to the #108 guard so the timeout is enforced locally and in CI, not just by CI job limits - Assert hn.last contains "Berg" in the #108 guard to catch silent last-name corruption - Add pytest-timeout dev dependency - Resolve pre-existing stash conflict in docs/resources.rst (keep upstream) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 7eb356d commit 2f4e126

4 files changed

Lines changed: 100 additions & 59 deletions

File tree

nameparser/parser.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -980,9 +980,11 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int =
980980
try:
981981
# if there are no more prefixes, look for a suffix to stop at
982982
stop_at = next(iter(filter(self.is_suffix, pieces[i + 1:])))
983-
# search from i + 1 so a repeated token earlier in the
984-
# list (e.g. a leading title that is also a suffix
985-
# acronym) is not matched instead of the trailing one
983+
# search from i + 1: filter() finds the value of stop_at
984+
# in pieces[i+1:] but pieces.index() without a start
985+
# argument searches from 0, so an earlier occurrence of
986+
# the same token (e.g. a suffix token that also appears
987+
# before the prefix) would be matched instead.
986988
j = pieces.index(stop_at, i + 1)
987989
new_piece = ' '.join(pieces[i:j])
988990
pieces = pieces[:i] + [new_piece] + pieces[j:]

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ dev = [
4343
"dill (>=0.2.5)",
4444
"sphinx (>=8)",
4545
"mypy (>=2.1)",
46-
"ruff (>=0.15)"
46+
"ruff (>=0.15)",
47+
"pytest-timeout>=2.4.0",
4748
]
4849

4950
[tool.mypy]

tests/test_prefixes.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import pytest
2+
13
from nameparser import HumanName
24

35
from tests.base import HumanNameTestBase
@@ -55,15 +57,37 @@ def test_title_before_and_after_prefixed_last_name(self) -> None:
5557
self.m(hn.last, "van Gogh", hn)
5658
self.m(hn.suffix, "dr", hn)
5759

60+
def test_suffix_token_collision_with_two_word_prefix(self) -> None:
61+
# Same fix as #100 but with a two-word prefix ("van der"). Exercises a
62+
# different iteration count through the prefix-joining loop.
63+
hn = HumanName("dr Vincent van der Gogh dr")
64+
self.m(hn.title, "dr", hn)
65+
self.m(hn.first, "Vincent", hn)
66+
self.m(hn.middle, "", hn)
67+
self.m(hn.last, "van der Gogh", hn)
68+
self.m(hn.suffix, "dr", hn)
69+
70+
def test_title_before_and_after_prefixed_last_name_with_middle(self) -> None:
71+
# The pre-fix bug corrupted the middle field; verify it is not disturbed
72+
# when a genuine middle name is present alongside the repeated token.
73+
hn = HumanName("dr Vincent James van Gogh dr")
74+
self.m(hn.title, "dr", hn)
75+
self.m(hn.first, "Vincent", hn)
76+
self.m(hn.middle, "James", hn)
77+
self.m(hn.last, "van Gogh", hn)
78+
self.m(hn.suffix, "dr", hn)
79+
80+
@pytest.mark.timeout(2)
5881
def test_many_repeated_prefixes_does_not_blow_up(self) -> None:
5982
# Issue #108: a name with a long run of repeated prefixes used to grow
60-
# the pieces list exponentially and exhaust memory. Guard against a
61-
# regression: this must parse quickly and not raise. If an exponential
62-
# code path is reintroduced, this test will hang (CI timeout catches it).
83+
# the pieces list exponentially and exhaust memory. The 2-second timeout
84+
# enforces this locally and in CI — if the test hangs, an exponential
85+
# regression has been reintroduced.
6386
name = "Jan " + "van der " * 30 + "Berg"
6487
hn = HumanName(name)
6588
self.assertFalse(hn.unparsable)
6689
self.m(hn.first, "Jan", hn)
90+
self.assertIn("Berg", hn.last)
6791

6892
def test_two_part_last_name_with_suffix_comma(self) -> None:
6993
hn = HumanName("pennie von bergen wessels, III")

uv.lock

Lines changed: 66 additions & 52 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)