Skip to content

Commit 89a88a6

Browse files
derek73claude
andcommitted
fix: address PR review feedback on suffix_delimiter
- Filter empty tokens from trailing/leading delimiters to prevent silent parse corruption (e.g. "MD-PhD-" with suffix_delimiter="-") - Clarify no-op note in Constants docstring: both the comma split and subsequent strip() make ", " a no-op, not just the comma split - Correct param docstring: expansion applies to all post-comma parts, not just identified suffix groups - Tighten known-limitation test with concrete field assertions instead of a fragile assertNotEqual - Add tests: trailing delimiter, comma-space no-op, inverted-format limitation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b2d9a9f commit 89a88a6

3 files changed

Lines changed: 28 additions & 7 deletions

File tree

nameparser/config/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,9 @@ class Constants:
235235
``"RN - CRNA"`` to be parsed as two separate suffixes. Default is
236236
``None`` (no additional splitting beyond the standard comma split).
237237
238-
Note: setting this to ``", "`` is a no-op — comma-splitting already
239-
occurs unconditionally before this step.
238+
Note: setting this to ``","`` or ``", "`` has no additional effect —
239+
the full name is already split on bare commas first, and each resulting
240+
part is stripped of surrounding whitespace before this step runs.
240241
241242
Known limitation: the expansion is applied to all post-comma parts, not
242243
just suffix groups. In inverted format (``"Last, First, suffix"``), the

nameparser/parser.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ class HumanName:
5454
:param str initials_format: python initials string formatting
5555
:param str initials_delimter: string delimiter for initials
5656
:param str initials_separator: string separator between consecutive initials
57-
:param str suffix_delimiter: additional delimiter to split suffix groups
58-
after comma-splitting, e.g. ``" - "`` for ``"RN - CRNA"``
57+
:param str suffix_delimiter: additional delimiter to split post-comma parts
58+
before suffix detection, e.g. ``" - "`` for ``"RN - CRNA"``
5959
:param str first: first name
6060
:param str middle: middle name
6161
:param str last: last name
@@ -631,7 +631,7 @@ def parse_full_name(self) -> None:
631631
if self.suffix_delimiter and len(parts) > 1:
632632
expanded = [parts[0]]
633633
for part in parts[1:]:
634-
expanded.extend([p.strip() for p in part.split(self.suffix_delimiter)])
634+
expanded.extend([p for p in (p.strip() for p in part.split(self.suffix_delimiter)) if p])
635635
parts = expanded
636636

637637
log.debug("full_name: %s", self._full_name)

tests/test_suffixes.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,26 @@ def test_suffix_delimiter_constants_level(self) -> None:
180180
def test_suffix_delimiter_none_by_default_known_limitation(self) -> None:
181181
# Without suffix_delimiter set, " - " between suffixes breaks parsing.
182182
# This test documents the known limitation — do not "fix" it.
183-
# (Passes when first is "RN" or empty — any incorrect parse is acceptable.)
184183
hn = HumanName("Steven Hardman, RN - CRNA")
185-
self.assertNotEqual(hn.first, "Steven")
184+
self.m(hn.first, "RN", hn)
185+
self.m(hn.last, "Steven Hardman", hn)
186+
self.m(hn.suffix, "CRNA", hn)
187+
188+
def test_suffix_delimiter_trailing_delimiter_ignored(self) -> None:
189+
# Trailing delimiter produces an empty token that must be filtered out.
190+
# Using a non-whitespace-terminated delimiter so stripping doesn't consume it.
191+
hn = HumanName("John Doe, MD-PhD-", suffix_delimiter="-")
192+
self.m(hn.first, "John", hn)
193+
self.m(hn.last, "Doe", hn)
194+
self.m(hn.suffix, "MD, PhD", hn)
195+
196+
def test_suffix_delimiter_comma_space_is_noop(self) -> None:
197+
hn = HumanName("John Doe, MD, PhD", suffix_delimiter=", ")
198+
self.m(hn.suffix, "MD, PhD", hn)
199+
200+
def test_suffix_delimiter_inverted_format_known_limitation(self) -> None:
201+
# In inverted format, the first-name part is also split on the delimiter.
202+
# "Mary - Kate" becomes two separate parts, causing a wrong parse.
203+
# This is a documented limitation — do not "fix" it without a broader solution.
204+
hn = HumanName("Doe, Mary - Kate, RN", suffix_delimiter=" - ")
205+
self.assertNotEqual(hn.first, "Mary - Kate")

0 commit comments

Comments
 (0)