FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Apply suffix_delimiter only at suffix-consumption sites by derek73 · Pull Request #206 · derek73/python-nameparser · GitHub

Apply suffix_delimiter only at suffix-consumption sites - #206

Merged
derek73 merged 2 commits into
masterfrom
fix/suffix-delimiter-consumption-site
Jul 4, 2026
Merged

Apply suffix_delimiter only at suffix-consumption sites#206
derek73 merged 2 commits into
masterfrom
fix/suffix-delimiter-consumption-site

Conversation

derek73 commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • suffix_delimiter previously split every post-comma segment unconditionally, before the parser decided which segment was actually a suffix group. This let it leak into first/middle-name segments in inverted format (e.g. "Doe, Mary - Kate, RN"), a case that was documented in Constants.suffix_delimiter's docstring as a known limitation.
  • Moved the delimiter split to the three places that actually consume a segment as suffixes: the suffix-comma vs. lastname-comma format detection (are_suffixes_after_comma), and the two suffix_list += parts[...] consumption points (suffix-comma branch's parts[1:], lastname-comma branch's parts[2:]).
  • The detection check now expands parts[1] on the delimiter to correctly recognize delimiter-joined suffixes (e.g. "RN - CRNA") while never expanding a segment that turns out to be a name.
  • Updated the docstring and replaced test_suffix_delimiter_inverted_format_known_limitation with test_suffix_delimiter_inverted_format_not_misparsed, asserting the corrected parse instead of just documenting the bug.

Test plan

  • pytest tests/test_suffixes.py -k suffix_delimiter -v — all 20 pass
  • Full suite: pytest -q — 1242 passed, 4 skipped, 22 xfailed, no regressions

derek73 added 2 commits July 4, 2026 02:36
Previously the delimiter split ran unconditionally on every post-comma
segment before the parser had decided which segment is actually a
suffix group. That let it leak into first/middle-name segments in
inverted format (e.g. "Doe, Mary - Kate, RN"), which was documented as
a known limitation.

Move the split to the three places that actually treat a segment as
suffixes: the suffix-comma vs. lastname-comma format detection, and
the two suffix_list += parts[...] consumption points. The detection
check now expands parts[1] to correctly recognize delimiter-joined
suffixes (e.g. "RN - CRNA") without ever expanding a segment that
turns out to be a name.
…ents

- Lock in hn.middle for the "Doe, Mary - Kate, RN" case so the stray
  hyphen artifact (a pre-existing tokenization quirk, reproducible
  without suffix_delimiter set) can't silently drift.
- Add coverage for the parts[1:] loop expanding more than one comma
  segment, for a multi-word token on one side of the delimiter in the
  detection check, and for delimiter no-op parity with the no-delimiter
  baseline when the format isn't detected as suffix-comma.
- Note in expand_suffix_delimiter's docstring that it's a no-op without
  a configured delimiter, and comment why detection needs the
  delimiter-expanded view of parts[1].
derek73 self-assigned this Jul 4, 2026
derek73 added this to the v1.3.0 milestone Jul 4, 2026
derek73 merged commit b64a44f into master Jul 4, 2026
8 checks passed
derek73 deleted the fix/suffix-delimiter-consumption-site branch July 4, 2026 09:46
derek73 added a commit that referenced this pull request Aug 24, 2026
The review round found the first draft shipped the inverse of the bug it
fixed. It let ANY piece open an entry, as the tail block always had --
safe there, because assign routes every tail piece to SUFFIX, which is
what `tail` means, and wrong off it, where a title piece routes to TITLE.

Two failures, one cause, neither visible to the gates that passed. The
`joined` tag is role-BLIND and the facade heals it for every role:

    "Smith, Rev. Dr."     title_list  ['Rev.','Dr.'] -> ['Rev. Dr.']
    "Smith Jr., Mr. Jr."  suffix      'Jr., Jr.'     -> 'Jr. Jr.'

The second glues a suffix backward across a comma the writer typed --
exactly what #429 exists to stop. The differential compares strings and
cannot see the first; the case table asserts the title STRING, which is
space-joined either way, and could not see it either.

Two joins that had been one, separated: WITHIN a piece the tag renders a
merged piece as one unit whatever role it holds; BETWEEN pieces it
continues an entry, and only a piece rendering into the same run may do
that. Sticky across a piece that is not in the entry, so an interleaved
title does not split its run ("Smith, MD Dr. PhD" -> 'MD PhD'); a
delimiter core still closes it.

Eight case rows and a facade test for the list views, which is the only
surface that shows the title collapse. Both regression guards verified
against a mutation copy -- they fail with the old condition restored.

Prose corrections, all measured by the reviewers:

- The round-trip claim was false AND backwards: str() of a fixed parse
  is a no-comma string, which re-parses with the comma back. master was
  the str-stable one. Struck from the release log and the case note.
- "one-word family comma" is not the condition -- there is no word-count
  gate, so "John Smith, Jr. III" moves too (1.4.0's reading), as does a
  title-led "Smith, Dr. MD PhD". Scope restated as it reads.
- The delimiter parity is #206 (021823e, "Apply suffix_delimiter only at
  suffix-consumption sites"), NOT #191, the German/Dutch vocabulary PR.
  Three code comments carried the error; corrected with it.
- The dormant-rule tell is #373's, and #426 the precedent for dropping a
  shadowed rule -- neither #424 entry mentions it.
- "boundary example" in the entry and both ledgers: the example FIRES,
  which is why the annotation came off.
- "filed rather than folded in" claimed an issue that does not exist.

C1 gains `_group.py` in `implemented:`, with the verbatim citation the
equality guard requires -- the whole-run half of the rule renders here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL