You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Five shared piece predicates live in _group.py only because assign imports group #439
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:
_is_suffix_piece
_peel_walk / _peel_trailing (the S2 trailing peel)
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.
Related: #438 names the pattern that produces these (five instances, and this is its accumulated cost). #437 would remove one of the five, so doing this move first makes that change cheaper.
Activity
derek73 commentedon Aug 25, 2026
Related: #438 names the pattern that produces these (five instances, and this is its accumulated cost). #437 would remove one of the five, so doing this move first makes that change cheaper.
test(layering): a pipeline module cannot skip the contract by omission
refactor(pipeline): the review round -- guards hardened, prose caught up
Merge pull request #441 from derek73/refactor/439-shared-piece-predic…
test(pieces): a unit-test home, and the branch no parse can reach
refactor(pipeline): piece-level predicates get their own module (dere…
docs(design): the piece layer has a module now (derek73#439)
test(benchmark,tools): a per-interpreter band, and a harness in the tree
test(benchmark): bound calls per parse, not seconds (derek73#475)