nameparser/_pipeline/_group.py is 1037 lines, and its module docstring now enumerates five predicates that live there for a reason that has nothing to do with grouping:
Each is there because _assign imports _group, so a predicate both stages ask cannot live in _assign. They belong to neither stage; together they are a shared "how does a piece read" vocabulary.
Why now
Four consecutive PRs have added to that list — #424, #425, #401/#421, #429 — and _group.py has become the default destination by import direction rather than by topic. The pattern that produces them is #438; this is its accumulated cost.
_vocab.py already hosts exactly this kind of thing: is_wholly_suffix was lifted there in #319 for the same reason, so the precedent and the destination both exist.
Proposal
Move the piece-level predicates out of _group.py into a module that both stages import — _vocab.py, or a new _pieces.py if the piece/tag layer is worth separating from the vocabulary layer.
Pure relocation: no behavior change, no ledger entries, no release-log bullet. The layering test (tests/v2/test_layering.py) already permits any nameparser._pipeline. import from a stage, and its comment currently has to name the exception explicitly — "stages share _state plus in-package helpers (_vocab, _group's piece predicates)" — which is the smell in one sentence.
Worth doing before, not after
#437 would delete group's only reason to read _segment_holds_no_name, and any future instance of #438 adds another predicate here. Doing the move first makes both cheaper, and it is the kind of change that only gets harder as the list grows.
Caution
git blame on the moved functions: this repo cares about the blame trail, and there is a recorded convention about splitting a rename from a shim into two commits so blame survives. A pure move with no edits should be one commit on its own, with no reformatting, so the follow-the-move blame works.
Came out of the cleanup review of #435.
nameparser/_pipeline/_group.pyis 1037 lines, and its module docstring now enumerates five predicates that live there for a reason that has nothing to do with grouping:_is_suffix_piece_peel_walk/_peel_trailing(the S2 trailing peel)_leading_titles_is_leading_title_segment_holds_no_name(added by parse("Smith, MD PhD").suffix returns "MD, PhD" — a comma the input never had #429)Each is there because
_assignimports_group, so a predicate both stages ask cannot live in_assign. They belong to neither stage; together they are a shared "how does a piece read" vocabulary.Why now
Four consecutive PRs have added to that list — #424, #425, #401/#421, #429 — and
_group.pyhas become the default destination by import direction rather than by topic. The pattern that produces them is #438; this is its accumulated cost._vocab.pyalready hosts exactly this kind of thing:is_wholly_suffixwas lifted there in #319 for the same reason, so the precedent and the destination both exist.Proposal
Move the piece-level predicates out of
_group.pyinto a module that both stages import —_vocab.py, or a new_pieces.pyif the piece/tag layer is worth separating from the vocabulary layer.Pure relocation: no behavior change, no ledger entries, no release-log bullet. The layering test (
tests/v2/test_layering.py) already permits anynameparser._pipeline.import from a stage, and its comment currently has to name the exception explicitly — "stages share_stateplus in-package helpers (_vocab,_group's piece predicates)" — which is the smell in one sentence.Worth doing before, not after
#437 would delete
group's only reason to read_segment_holds_no_name, and any future instance of #438 adds another predicate here. Doing the move first makes both cheaper, and it is the kind of change that only gets harder as the list grows.Caution
git blameon the moved functions: this repo cares about the blame trail, and there is a recorded convention about splitting a rename from a shim into two commits so blame survives. A pure move with no edits should be one commit on its own, with no reformatting, so the follow-the-move blame works.Came out of the cleanup review of #435.