Apply suffix_delimiter only at suffix-consumption sites#206
Merged
Conversation
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].
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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
suffix_delimiterpreviously 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 inConstants.suffix_delimiter's docstring as a known limitation.are_suffixes_after_comma), and the twosuffix_list += parts[...]consumption points (suffix-comma branch'sparts[1:], lastname-comma branch'sparts[2:]).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.test_suffix_delimiter_inverted_format_known_limitationwithtest_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 passpytest -q— 1242 passed, 4 skipped, 22 xfailed, no regressions