From d6654575256a6647941e131b0a9df74a34070d46 Mon Sep 17 00:00:00 2001 From: akamick86 <12109250+akamick86@users.noreply.github.com> Date: Sat, 26 Sep 2026 09:12:55 -0700 Subject: [PATCH 1/2] perf(pipeline): copy stage state without dataclasses.replace Every stage returns a copy of the frozen ParseState, and classify, assign, group and post_rules also copy tokens one at a time. All of those went through dataclasses.replace, which walks fields() and calls __init__ on each copy: 18 copies per parse of the reference name, at three frames each on 3.11/3.12 and four from 3.13. copy_with in _pipeline/_state.py copies the fields directly. For a dataclass whose generated __init__ only assigns its fields that builds the same object, and WorkToken, ParseState and PendingAmbiguity are all that kind. It checks this once per class and refuses a class with __post_init__ or an init=False field. Parse cost drops 36 calls on 3.11/3.12 and 54 from 3.13, so the _CALL_BASELINE rows and the 3.11 _LINK_BASELINE row move down, each re-measured on its own interpreter (decisions.md#parse-cost). Differential gate output is unchanged at all five baselines. --- docs/design/decisions.md | 1 + docs/release_log.rst | 2 + nameparser/_pipeline/_assign.py | 9 ++--- nameparser/_pipeline/_classify.py | 7 ++-- nameparser/_pipeline/_extract.py | 5 +-- nameparser/_pipeline/_group.py | 9 ++--- nameparser/_pipeline/_post_rules.py | 21 +++++----- nameparser/_pipeline/_script_segment.py | 11 +++--- nameparser/_pipeline/_segment.py | 11 +++--- nameparser/_pipeline/_state.py | 45 +++++++++++++++++++++- nameparser/_pipeline/_tokenize.py | 7 ++-- tests/v2/pipeline/test_state.py | 51 ++++++++++++++++++++++++- tests/v2/test_benchmark.py | 19 ++++++--- 13 files changed, 145 insertions(+), 53 deletions(-) diff --git a/docs/design/decisions.md b/docs/design/decisions.md index 621a6b35..66bbbe9f 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -1433,6 +1433,7 @@ Every number below is a py3.11 measurement of 2026-08-31, recomputable with `uv ONE REPAIR WAS TRIED AND REVERTED: collapsing `post_rules`' three separate role-index scans into one pass. It saves **6 calls per parse** on 3.11 (the first draft said one, which is not even reachable — `_idx` is a one-line list comprehension, so each removed call frees two frames). Reverted because it was measured against a noisy timing harness that showed nothing; at 6 calls it is 9% of the cycle's growth and worth reconsidering against the band, which can now see it. RAISING OR LOWERING A ROW IS A DECISION, not a maintenance chore. Append here with the interpreter and the harness invocation, as above. +- 2026-09-26 #546 -- the stages copy their state with `_state.copy_with` instead of `dataclasses.replace`, and every row moves DOWN. `dataclasses.replace` walks `fields()` and calls `__init__` on each copy, three frames on 3.11 and 3.12 and four from 3.13 where `copy_with` is one, and one parse of the reference name makes 18 copies: every stage returns a copy of the state, and classify, assign, group and post_rules also copy tokens one at a time. That is the whole of the drop, 18 x 2 = 36 frames on 3.11 and 3.12 and 18 x 3 = 54 from 3.13 (recompute: wrap `copy_with` with a counter in the eight stage modules and parse the reference name). For a dataclass whose generated `__init__` only assigns its fields, which all three pipeline classes are (`WorkToken`, `ParseState`, `PendingAmbiguity`), a field copy builds the same object. `copy_with` checks that once per class, refuses a class that validates in `__post_init__` or carries an `init=False` field, and raises `TypeError` for an unknown field as `replace` does. Measured 2026-09-26 with `uv run python tools/perf/call_count.py --against e0f1a2f`, each row on its own interpreter, parse/facade: 3.11 406/443 → 370/407, 3.12 384/421 → 348/385, 3.13, 3.14 and 3.15 402/439 → 348/385. `_LINK_BASELINE`'s 64-link clause reads 2587 → 2301 on 3.11. By stage (`--stages`, py3.11, ms per 1000 parses of the reference name): group 19.6 → 17.3, classify 13.4 → 9.8, assign 11.1 → 8.0, tokenize 8.1 → 6.6, post_rules 7.9 → 6.5, segment 2.6 → 1.7. BEHAVIOR IDENTICAL: the differential gate's report at all five baselines matches e0f1a2f's line for line apart from the path header. ### removed-v1-surface diff --git a/docs/release_log.rst b/docs/release_log.rst index 5aba1ab3..169a082e 100644 --- a/docs/release_log.rst +++ b/docs/release_log.rst @@ -36,6 +36,8 @@ Release Log - **Change case repair to write an unlisted dotted credential and a roman numeral past iv in capitals.** ``HumanName("john smith x.y.z.").capitalize()`` gives ``John Smith X.Y.Z.`` where every release gave ``John Smith X.y.z.``, the dotted word being a suffix now (the ``unlisted_dotted_suffixes`` change above) and repaired as a listed acronym is; and ``john smith vi`` gives ``John Smith VI`` where every release gave ``John Smith Vi``, with ``vii``, ``viii`` and ``ix`` alike. Both are keyed on the suffix role: ``Jack X.Y.Z.``, which keeps its surname, still repairs as a name word (``Jack X.y.z.`` under ``force=True``), and ``john smith xi`` still gives ``John Smith Xi``, the parser reading ``xi`` as the surname. An unlisted dotted credential written in mixed case is kept as written on the default path, by the suffix change above (``john smith B.Tech.`` gives ``John Smith B.Tech.``), and reads all capitals under ``force=True`` (``John Smith B.TECH.``, where every release gave ``John Smith B.tech.``), which a ``capitalization_exceptions`` mask such as ``{"btech": "BTech"}`` undoes. Over the 1340 names in the differential corpora at the commit before this change (2026-09-23), 1 moves on the default path and 14 under ``force=True``. The recipe is the ``R4`` entry's 2026-09-23 MEASURED bullet in ``docs/design/decisions.md`` (#459) + - **Change the parse pipeline to copy its state without dataclasses.replace, about 10% faster per name.** Every stage returns a copy of its frozen state, and several copy tokens one at a time; ``dataclasses.replace`` goes through ``fields()`` and ``__init__`` on every one of those copies. The stages now copy fields directly through a small helper that refuses any class where that would differ from ``replace``. On py3.11 ``parse`` drops from 406 to 370 calls per parse of the benchmark's reference name and ``HumanName`` from 443 to 407, and the call-count baselines move with them. Recomputable with ``uv run python tools/perf/call_count.py --against e0f1a2f``; the counts for every interpreter are in the ``parse-cost`` entry of ``docs/design/decisions.md``. No user-visible behavior changes (#546) + **Additions** - **Add Lexicon.conjunctions_ambiguous, the one-letter connectives that read as initials.** A subset of ``conjunctions`` holding ``e`` and ``i`` by default; it is the knob for the change above rather than a switch. Portuguese data, where ``e`` links surnames the way ``y`` does in Spanish, takes it out: ``Lexicon.default().remove(conjunctions_ambiguous={"e"})`` restores the joining reading. Dutch data, where a bare single letter is an initial and never a connective, adds the other one: ``Lexicon.default().add(conjunctions_ambiguous={"y"})``. A v1 ``Constants`` has no manager of its own for it -- deleting the word from ``conjunctions`` is what turns the marking off, the same rule the glued-honorific tails follow. See ``docs/customize.rst`` (#383, #479) diff --git a/nameparser/_pipeline/_assign.py b/nameparser/_pipeline/_assign.py index 90f2e596..1e7663b1 100644 --- a/nameparser/_pipeline/_assign.py +++ b/nameparser/_pipeline/_assign.py @@ -63,7 +63,6 @@ """ from __future__ import annotations -import dataclasses from collections.abc import Sequence, Set from typing import NamedTuple @@ -78,7 +77,7 @@ ) from nameparser._pipeline._state import ( AMBIGUOUS_ACRONYM_TAG, ParseState, PendingAmbiguity, Structure, - WorkToken, _AMBIGUOUS_CREDENTIAL_TAGS, _NEVER_FLIPPED, + WorkToken, _AMBIGUOUS_CREDENTIAL_TAGS, _NEVER_FLIPPED, copy_with, ) from nameparser._policy import Policy, Script from nameparser._types import AmbiguityKind, Role @@ -86,7 +85,7 @@ def _set_roles(tokens: list[WorkToken], piece: tuple[int, ...], role: Role) -> None: for i in piece: - tokens[i] = dataclasses.replace(tokens[i], role=role) + tokens[i] = copy_with(tokens[i], role=role) #: Tags that say the word's own reading was claimed before position @@ -672,7 +671,7 @@ def previous_kept(m: int, titled: tuple[int, ...]) -> int: #: reads `.role`, verified by reading all three #: (2026-09-19). The one thing this segment's code rewrites #: between the two passes is the role, through `_set_roles`, - #: which is a `dataclasses.replace(role=...)` and leaves + #: which is a `copy_with(role=...)` and leaves #: text and tags identical. floors: dict[tuple[int, ...], tuple[int, bool]] = {} @@ -1024,6 +1023,6 @@ def reads_as_a_suffix(m: int, titled: tuple[int, ...]) -> bool: for seg_idx in range(tail, len(state.segments)): for piece in state.pieces[seg_idx]: _set_roles(tokens, piece, Role.SUFFIX) - return dataclasses.replace(state, tokens=tuple(tokens), + return copy_with(state, tokens=tuple(tokens), order=order, ambiguities=tuple(ambiguities)) diff --git a/nameparser/_pipeline/_classify.py b/nameparser/_pipeline/_classify.py index 27e6d7a8..a94e3256 100644 --- a/nameparser/_pipeline/_classify.py +++ b/nameparser/_pipeline/_classify.py @@ -49,12 +49,11 @@ """ from __future__ import annotations -import dataclasses from nameparser._lexicon import _normalize from nameparser._pipeline._state import ( AMBIGUOUS_ACRONYM_TAG, SHAPE_ACRONYM_TAG, ParseState, PendingAmbiguity, - WorkToken, + WorkToken, copy_with, ) from nameparser._types import AmbiguityKind, Role from nameparser._pipeline._vocab import ( @@ -264,7 +263,7 @@ def classify(state: ParseState) -> ParseState: # class they consult. No extra frame -- it is one more boolean in a # comprehension that already walks every token. tokens = tuple( - dataclasses.replace( + copy_with( t, tags=_tags_for(t, folded[i], state, marker_tags.get(i), one_case_own=one_case and i < clause_at and t.role is None, one_case=one_case)) @@ -324,6 +323,6 @@ def classify(state: ParseState) -> ParseState: (i,))) # The write rides the replace this stage already makes, so # recording the fact costs no frame of its own. - return dataclasses.replace(state, tokens=tokens, + return copy_with(state, tokens=tokens, ambiguities=tuple(ambiguities), one_case=one_case) diff --git a/nameparser/_pipeline/_extract.py b/nameparser/_pipeline/_extract.py index a0f365dd..239713a3 100644 --- a/nameparser/_pipeline/_extract.py +++ b/nameparser/_pipeline/_extract.py @@ -32,12 +32,11 @@ from __future__ import annotations import bisect -import dataclasses import functools from nameparser._lexicon import Lexicon, _normalize from nameparser._pipeline._state import ( - COMMA_CHARS, ParseState, PendingAmbiguity, + COMMA_CHARS, ParseState, PendingAmbiguity, copy_with, ) from nameparser._pipeline._vocab import maiden_marker_run from nameparser._types import AmbiguityKind, Role, Span @@ -298,6 +297,6 @@ def extract_delimited(state: ParseState) -> ParseState: continue reported.add(j) ambiguities.append(_unmatched(close, j)[1]) - return dataclasses.replace( + return copy_with( state, extracted=tuple(extracted), masked=tuple(masked), ambiguities=state.ambiguities + tuple(ambiguities)) diff --git a/nameparser/_pipeline/_group.py b/nameparser/_pipeline/_group.py index b66f6983..e33e0782 100644 --- a/nameparser/_pipeline/_group.py +++ b/nameparser/_pipeline/_group.py @@ -38,7 +38,6 @@ from __future__ import annotations import bisect -import dataclasses from collections.abc import Iterable, Sequence, Set from enum import IntEnum from typing import assert_never @@ -52,7 +51,7 @@ ) from nameparser._pipeline._state import ( AMBIGUOUS_ACRONYM_TAG, ParseState, PendingAmbiguity, Structure, - WorkToken, _AMBIGUOUS_CREDENTIAL_TAGS, + WorkToken, _AMBIGUOUS_CREDENTIAL_TAGS, copy_with, ) from nameparser._pipeline._vocab import D, PH from nameparser._pipeline._vocab import delimiter_cores @@ -1687,7 +1686,7 @@ def group(state: ParseState) -> ParseState: dropped.extend(marker_piece) for piece in maiden_pieces: for i in piece: - tokens[i] = dataclasses.replace( + tokens[i] = copy_with( tokens[i], role=Role.MAIDEN) # rules.md#C1: "a part that is nothing but suffix words is the # credential run and reads as suffixes, whole" -- WHOLE is this @@ -1737,7 +1736,7 @@ def group(state: ParseState) -> ParseState: for piece, piece_tags_ in zip(pieces, ptags): if "suffix" in piece_tags_ and len(piece) > 1: for i in piece[1:]: - tokens[i] = dataclasses.replace( + tokens[i] = copy_with( tokens[i], tags=tokens[i].tags | {"joined"}) all_pieces.append(tuple(tuple(p) for p in pieces)) all_ptags.append(tuple(frozenset(t) for t in ptags)) @@ -1816,7 +1815,7 @@ def group(state: ParseState) -> ParseState: if (first + run < len(tokens) and tokens[first + run].span.end <= clause.end): dropped.extend(range(first, first + run)) - return dataclasses.replace( + return copy_with( state, tokens=tuple(tokens), pieces=tuple(all_pieces), piece_tags=tuple(all_ptags), dropped=tuple(dropped), ambiguities=tuple(ambiguities)) diff --git a/nameparser/_pipeline/_post_rules.py b/nameparser/_pipeline/_post_rules.py index b9c5d1fb..fab443b2 100644 --- a/nameparser/_pipeline/_post_rules.py +++ b/nameparser/_pipeline/_post_rules.py @@ -20,14 +20,13 @@ """ from __future__ import annotations -import dataclasses import re from nameparser._lexicon import _run_addresses_by_given from nameparser._pipeline._assign import _name_positions from nameparser._pipeline._state import ( AMBIGUOUS_ACRONYM_TAG, ParseState, PendingAmbiguity, Structure, - WorkToken, _NEVER_FLIPPED, comma_bucket, + WorkToken, _NEVER_FLIPPED, comma_bucket, copy_with, ) from nameparser._pipeline._vocab import delimiter_cores from nameparser._policy import PatronymicRule @@ -193,7 +192,7 @@ def _mark_suffix_entries(tokens: list[WorkToken], state: ParseState) -> None: else tokens[between].role not in _RENDERS_ELSEWHERE for between in range(previous + 1, current)) if same_part and not parted: - tokens[current] = dataclasses.replace( + tokens[current] = copy_with( tokens[current], tags=tokens[current].tags | {"joined"}) @@ -214,7 +213,7 @@ def suffix_entries(state: ParseState) -> ParseState: nothing else.""" tokens = list(state.tokens) _mark_suffix_entries(tokens, state) - return dataclasses.replace(state, tokens=tuple(tokens)) + return copy_with(state, tokens=tuple(tokens)) def _idx(tokens: list[WorkToken], role: Role) -> list[int]: @@ -247,7 +246,7 @@ def _leading_name_piece(state: ParseState, def _retag(tokens: list[WorkToken], i: int, role: Role) -> None: - tokens[i] = dataclasses.replace(tokens[i], role=role) + tokens[i] = copy_with(tokens[i], role=role) # rules.md#P2: "a particle joins the words after it into one name @@ -652,7 +651,7 @@ def post_rules(state: ParseState) -> ParseState: f"of its own", tuple(sorted(run)))) for j in run: - tokens[j] = dataclasses.replace( + tokens[j] = copy_with( tokens[j], role=Role.FAMILY, tags=tokens[j].tags | {FOLDED_TAG}) # recomputed for H1's reason, stated at H1: a stale index @@ -811,7 +810,7 @@ def post_rules(state: ParseState) -> ParseState: f"rather than standing as a name word of its own", tuple(run))) for i in run: - tokens[i] = dataclasses.replace( + tokens[i] = copy_with( tokens[i], role=Role.FAMILY, tags=tokens[i].tags | {FOLDED_TAG}) @@ -822,7 +821,7 @@ def post_rules(state: ParseState) -> ParseState: # tags the token, and the rendering views consult the tag" if state.policy.middle_as_family: for i in _idx(tokens, Role.MIDDLE): - tokens[i] = dataclasses.replace( + tokens[i] = copy_with( tokens[i], role=Role.FAMILY, tags=tokens[i].tags | {FOLDED_TAG}) # rules.md#R2: "a name part whose every word is particle @@ -874,7 +873,7 @@ def post_rules(state: ParseState) -> ParseState: others += 1 if all_particle: for i in part: - tokens[i] = dataclasses.replace( + tokens[i] = copy_with( tokens[i], tags=tokens[i].tags | {UNJOINED_TAG}) elif conj and not others: # #461: nothing here for the connective to join. The `elif` @@ -883,9 +882,9 @@ def post_rules(state: ParseState) -> ParseState: # and connective included, which is what keeps a caller's # `add(particles={"y"})` readings unchanged. for i in conj: - tokens[i] = dataclasses.replace( + tokens[i] = copy_with( tokens[i], tags=tokens[i].tags | {UNJOINED_CONJUNCTION_TAG}) _mark_suffix_entries(tokens, state) - return dataclasses.replace(state, tokens=tuple(tokens), + return copy_with(state, tokens=tuple(tokens), ambiguities=tuple(ambiguities)) diff --git a/nameparser/_pipeline/_script_segment.py b/nameparser/_pipeline/_script_segment.py index f5f60cc9..bc94f714 100644 --- a/nameparser/_pipeline/_script_segment.py +++ b/nameparser/_pipeline/_script_segment.py @@ -58,13 +58,12 @@ """ from __future__ import annotations -import dataclasses import functools from collections.abc import Sequence from nameparser._lexicon import FULL_STOPS from nameparser._pipeline._state import ( - ParseState, PendingAmbiguity, Structure, WorkToken, + ParseState, PendingAmbiguity, Structure, WorkToken, copy_with, ) from nameparser._pipeline._vocab import ( effective_script, is_suffix_strict, is_wholly_suffix, @@ -157,11 +156,11 @@ def _split(state: ParseState, i: int, splits: tuple[int, ...], start = 0 for piece in _pieces(token.text, splits): end = start + len(piece) - parts.append(dataclasses.replace( + parts.append(copy_with( token, text=piece, span=Span(base + start, base + end))) start = end if tail_tag is not None: - parts[-1] = dataclasses.replace( + parts[-1] = copy_with( parts[-1], tags=parts[-1].tags | {tail_tag}) added = len(splits) tokens = state.tokens[:i] + tuple(parts) + state.tokens[i + 1:] @@ -172,14 +171,14 @@ def _split(state: ParseState, i: int, splits: tuple[int, ...], # pointing at the head. segments = tuple(_remap(run, i, added) for run in state.segments) ambiguities = tuple( - dataclasses.replace(a, indices=tuple( + copy_with(a, indices=tuple( j + added if j > i else j for j in a.indices)) for a in state.ambiguities) if detail is not None: ambiguities += (PendingAmbiguity( AmbiguityKind.SEGMENTATION, detail, tuple(range(i, i + added + 1))),) - return dataclasses.replace(state, tokens=tokens, segments=segments, + return copy_with(state, tokens=tokens, segments=segments, ambiguities=ambiguities) diff --git a/nameparser/_pipeline/_segment.py b/nameparser/_pipeline/_segment.py index 8ab71c6d..98663431 100644 --- a/nameparser/_pipeline/_segment.py +++ b/nameparser/_pipeline/_segment.py @@ -37,11 +37,10 @@ """ from __future__ import annotations -import dataclasses from nameparser._pipeline._pieces import own_words from nameparser._pipeline._state import ( - ParseState, PendingAmbiguity, Structure, comma_bucket, + ParseState, PendingAmbiguity, Structure, comma_bucket, copy_with, ) from nameparser._pipeline._vocab import ( ambiguous_class_candidate, ambiguous_class_member, caps_shape_candidate, @@ -56,10 +55,10 @@ def segment(state: ParseState) -> ParseState: main = [i for i, t in enumerate(state.tokens) if t.role is None] if not main: - return dataclasses.replace(state, segments=(), + return copy_with(state, segments=(), structure=Structure.NO_COMMA) if not state.comma_offsets: - return dataclasses.replace(state, segments=(tuple(main),), + return copy_with(state, segments=(tuple(main),), structure=Structure.NO_COMMA) buckets: list[list[int]] = [[] for _ in range(len(state.comma_offsets) + 1)] for i in main: @@ -79,7 +78,7 @@ def segment(state: ParseState) -> ParseState: groups.pop() if len(groups) <= 1: segs = tuple(groups) if groups and groups[0] else (tuple(main),) - return dataclasses.replace(state, segments=segs, + return copy_with(state, segments=segs, structure=Structure.NO_COMMA) # The case fact, asked LAZILY: only a comma form can turn on it @@ -306,7 +305,7 @@ def class_run(seg: tuple[int, ...]) -> bool: f"segment {texts_joined!r} beyond the recognized comma " f"structures; consumed as suffix best-effort", tuple(seg))) - return dataclasses.replace(state, segments=tuple(groups), + return copy_with(state, segments=tuple(groups), structure=structure, ambiguities=tuple(ambiguities), one_case=one_case) diff --git a/nameparser/_pipeline/_state.py b/nameparser/_pipeline/_state.py index 95e70b7f..f11cb2db 100644 --- a/nameparser/_pipeline/_state.py +++ b/nameparser/_pipeline/_state.py @@ -11,9 +11,11 @@ from __future__ import annotations import bisect +import dataclasses from collections.abc import Sequence from dataclasses import dataclass from enum import Enum, auto +from typing import TypeVar from nameparser._lexicon import Lexicon from nameparser._policy import Policy @@ -21,6 +23,47 @@ Role, Segmenter, Span) +_T = TypeVar("_T") + +#: Field names per class, recorded the first time `copy_with` copies one. +_COPY_FIELDS: dict[type, tuple[str, ...]] = {} + + +def copy_with(obj: _T, /, **changes: object) -> _T: + """`dataclasses.replace` for the pipeline's own dataclasses, without + its per-call cost. + + Every stage returns a copy of the state, and several copy tokens one + at a time, so this runs many times per parse. The stdlib replace + walks `fields()` and goes through `__init__` on every call; for a + class whose `__init__` only assigns its fields, copying the fields + directly gives the same object. `_copy_fields` checks that once per + class and refuses any class where it would not hold + (decisions.md#parse-cost has the measurement). + """ + cls = type(obj) + names = _COPY_FIELDS.get(cls) or _copy_fields(cls) + new = object.__new__(cls) + for name in names: + value = changes.pop(name) if name in changes else getattr(obj, name) + object.__setattr__(new, name, value) + if changes: + raise TypeError( + f"{cls.__name__} has no field named {', '.join(sorted(changes))}") + return new + + +def _copy_fields(cls: type) -> tuple[str, ...]: + params = getattr(cls, "__dataclass_params__", None) + if (params is None or not params.init or hasattr(cls, "__post_init__") + or not all(f.init for f in dataclasses.fields(cls))): + raise TypeError( + f"copy_with cannot copy {cls.__name__}: it needs a dataclass " + "whose generated __init__ only assigns its fields") + names = _COPY_FIELDS[cls] = tuple(f.name for f in dataclasses.fields(cls)) + return names + + # The comma characters (ASCII/Arabic/fullwidth, #265). Shared here so # tokenize (separators/segmentation) and extract (close-quote # boundaries) cannot drift apart. @@ -126,7 +169,7 @@ class PendingAmbiguity: @dataclass(frozen=True, slots=True) class ParseState: """Carried through the stage fold. Frozen; stages return copies via - dataclasses.replace. Fields are filled progressively: + copy_with. Fields are filled progressively: extract_delimited -> extracted/masked; tokenize -> tokens (span- sorted)/comma_offsets/interpunct_offsets (the 间隔号 offsets the order and segmentation decisions consult, #298; the nakaguro diff --git a/nameparser/_pipeline/_tokenize.py b/nameparser/_pipeline/_tokenize.py index 94770469..fd8ce3b1 100644 --- a/nameparser/_pipeline/_tokenize.py +++ b/nameparser/_pipeline/_tokenize.py @@ -15,11 +15,10 @@ from __future__ import annotations import bisect -import dataclasses import re from nameparser._pipeline._state import ( - COMMA_CHARS, ParseState, WorkToken, + COMMA_CHARS, ParseState, WorkToken, copy_with, ) from nameparser._policy import Policy, _SCRIPT_RANGES, _script_matcher from nameparser._types import Role, Span @@ -189,9 +188,9 @@ def _containing(offset: int) -> tuple[int, ...]: ambiguities = tuple( a if a.origin is None - else dataclasses.replace(a, indices=_containing(a.origin)) + else copy_with(a, indices=_containing(a.origin)) for a in ambiguities) - return dataclasses.replace(state, tokens=tuple(tokens), + return copy_with(state, tokens=tuple(tokens), comma_offsets=tuple(sorted(commas)), interpunct_offsets=tuple(sorted(interpuncts)), ambiguities=ambiguities) diff --git a/tests/v2/pipeline/test_state.py b/tests/v2/pipeline/test_state.py index 664d04d3..dbfe05bf 100644 --- a/tests/v2/pipeline/test_state.py +++ b/tests/v2/pipeline/test_state.py @@ -1,9 +1,13 @@ import dataclasses +import pytest + from nameparser._lexicon import Lexicon -from nameparser._pipeline._state import ParseState, Structure, WorkToken +from nameparser._pipeline._state import ( + ParseState, PendingAmbiguity, Structure, WorkToken, copy_with, +) from nameparser._policy import Policy -from nameparser._types import Role, Span +from nameparser._types import AmbiguityKind, Role, Span def _state(text: str) -> ParseState: @@ -29,6 +33,49 @@ def test_state_is_frozen_and_replace_works() -> None: assert s2.tokens[0].role is None and s2.tokens[0].tags == frozenset() +def test_copy_with_builds_what_dataclasses_replace_builds() -> None: + # copy_with stands in for dataclasses.replace on every pipeline + # dataclass, so the two have to agree on each of them. + tok = WorkToken("x", Span(0, 1)) + pending = PendingAmbiguity(AmbiguityKind.COMMA_STRUCTURE, "detail") + for obj, changes in ( + (_state("x"), {"tokens": (tok,), "one_case": True}), + (tok, {"role": Role.GIVEN, "tags": frozenset({"initial"})}), + (pending, {"indices": (0,)}), + ): + assert copy_with(obj, **changes) == dataclasses.replace(obj, **changes) + assert copy_with(obj) == obj and copy_with(obj) is not obj + + +def test_copy_with_rejects_a_field_the_class_does_not_have() -> None: + with pytest.raises(TypeError, match="no field named rol"): + copy_with(WorkToken("x", Span(0, 1)), rol=Role.GIVEN) + + +@dataclasses.dataclass(frozen=True) +class _Validated: + value: int + + def __post_init__(self) -> None: + if self.value < 0: + raise ValueError("negative") + + +@dataclasses.dataclass(frozen=True) +class _Derived: + value: int + doubled: int = dataclasses.field(init=False, default=0) + + +@pytest.mark.parametrize("obj", [_Validated(1), _Derived(1)]) +def test_copy_with_refuses_a_class_whose_init_does_more_than_assign( + obj: object) -> None: + # Copying fields would skip the validation or recompute the + # init=False field that dataclasses.replace goes through __init__ for. + with pytest.raises(TypeError, match="cannot copy"): + copy_with(obj, value=2) + + def test_worktoken_carries_optional_role() -> None: t = WorkToken("Jack", Span(6, 10), role=Role.NICKNAME) assert t.role is Role.NICKNAME diff --git a/tests/v2/test_benchmark.py b/tests/v2/test_benchmark.py index 913d572f..ab416f69 100644 --- a/tests/v2/test_benchmark.py +++ b/tests/v2/test_benchmark.py @@ -59,12 +59,16 @@ #: are the two interpreters on the author's machine; 3.12, 3.13 and #: 3.15 are CI's, seeded from a review measurement and confirmed by #: the first green run -- a wrong seed fails with the real number. +#: +#: Lowered 2026-09-26 when the stages stopped copying state through +#: dataclasses.replace (decisions.md#parse-cost). All five rows were +#: re-measured with that harness, each on its own interpreter. _CALL_BASELINE = { - (3, 11): {"parse": 410, "facade": 447}, - (3, 12): {"parse": 388, "facade": 425}, - (3, 13): {"parse": 406, "facade": 443}, - (3, 14): {"parse": 406, "facade": 443}, - (3, 15): {"parse": 406, "facade": 443}, + (3, 11): {"parse": 370, "facade": 407}, + (3, 12): {"parse": 348, "facade": 385}, + (3, 13): {"parse": 348, "facade": 385}, + (3, 14): {"parse": 348, "facade": 385}, + (3, 15): {"parse": 348, "facade": 385}, } _BAND = 0.02 @@ -596,8 +600,11 @@ def test_a_clause_link_run_does_not_cost_quadratically() -> None: # are NOT recorded below, because recording them would put a number # under a band without a run behind it. Reproduce one on its own # interpreter and add the row. +# +# Lowered 2026-09-26 from 2587 with `_CALL_BASELINE` above, for the +# same reason (decisions.md#parse-cost). _LINK_BASELINE = { - (3, 11): 2587, + (3, 11): 2301, } #: The same +-2% `_CALL_BASELINE` uses, and for the same reason: frame #: counts are deterministic for a given tree and interpreter, so the From a2edd3f1e1e5b5914e80e07d903f29d8600a48de Mon Sep 17 00:00:00 2001 From: akamick86 <12109250+akamick86@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:33:33 -0700 Subject: [PATCH 2/2] perf(pipeline): address review -- typed copy_with, import-time table, exact guard - mypy sees copy_with as `from dataclasses import replace as copy_with`, so the dataclass plugin checks its keywords again. An assignment (`copy_with = dataclasses.replace`) does not: the plugin keys on the callee's full name, and a misspelled or wrong-typed field passes. - _COPY_FIELDS is a read-only table built at import over the three pipeline classes, replacing the lazily filled dict. copy_with refuses any other class, and the zero-field falsy-cache wrinkle is gone. - _copyable_fields checks the class's own __dataclass_params__ and a generated __init__, so a validating __init__ and an undecorated subclass are refused. The redundant params.init test is dropped. - The refusal test records what replace builds and what an unguarded copy would build for each refused shape; each guard clause fails its own row when removed. - Release log states the saving in calls; the decisions entry gives the 6 + 12 copy breakdown and why the rows drop by 40 and 58. - Continuation lines realigned, _segment's double blank line removed. --- docs/design/decisions.md | 2 +- docs/release_log.rst | 2 +- nameparser/_pipeline/_assign.py | 4 +- nameparser/_pipeline/_classify.py | 4 +- nameparser/_pipeline/_post_rules.py | 2 +- nameparser/_pipeline/_script_segment.py | 2 +- nameparser/_pipeline/_segment.py | 13 ++- nameparser/_pipeline/_state.py | 100 ++++++++++++++---------- nameparser/_pipeline/_tokenize.py | 6 +- tests/v2/pipeline/test_state.py | 86 ++++++++++++++++++-- 10 files changed, 152 insertions(+), 69 deletions(-) diff --git a/docs/design/decisions.md b/docs/design/decisions.md index 66bbbe9f..4ae36ebb 100644 --- a/docs/design/decisions.md +++ b/docs/design/decisions.md @@ -1433,7 +1433,7 @@ Every number below is a py3.11 measurement of 2026-08-31, recomputable with `uv ONE REPAIR WAS TRIED AND REVERTED: collapsing `post_rules`' three separate role-index scans into one pass. It saves **6 calls per parse** on 3.11 (the first draft said one, which is not even reachable — `_idx` is a one-line list comprehension, so each removed call frees two frames). Reverted because it was measured against a noisy timing harness that showed nothing; at 6 calls it is 9% of the cycle's growth and worth reconsidering against the band, which can now see it. RAISING OR LOWERING A ROW IS A DECISION, not a maintenance chore. Append here with the interpreter and the harness invocation, as above. -- 2026-09-26 #546 -- the stages copy their state with `_state.copy_with` instead of `dataclasses.replace`, and every row moves DOWN. `dataclasses.replace` walks `fields()` and calls `__init__` on each copy, three frames on 3.11 and 3.12 and four from 3.13 where `copy_with` is one, and one parse of the reference name makes 18 copies: every stage returns a copy of the state, and classify, assign, group and post_rules also copy tokens one at a time. That is the whole of the drop, 18 x 2 = 36 frames on 3.11 and 3.12 and 18 x 3 = 54 from 3.13 (recompute: wrap `copy_with` with a counter in the eight stage modules and parse the reference name). For a dataclass whose generated `__init__` only assigns its fields, which all three pipeline classes are (`WorkToken`, `ParseState`, `PendingAmbiguity`), a field copy builds the same object. `copy_with` checks that once per class, refuses a class that validates in `__post_init__` or carries an `init=False` field, and raises `TypeError` for an unknown field as `replace` does. Measured 2026-09-26 with `uv run python tools/perf/call_count.py --against e0f1a2f`, each row on its own interpreter, parse/facade: 3.11 406/443 → 370/407, 3.12 384/421 → 348/385, 3.13, 3.14 and 3.15 402/439 → 348/385. `_LINK_BASELINE`'s 64-link clause reads 2587 → 2301 on 3.11. By stage (`--stages`, py3.11, ms per 1000 parses of the reference name): group 19.6 → 17.3, classify 13.4 → 9.8, assign 11.1 → 8.0, tokenize 8.1 → 6.6, post_rules 7.9 → 6.5, segment 2.6 → 1.7. BEHAVIOR IDENTICAL: the differential gate's report at all five baselines matches e0f1a2f's line for line apart from the path header. +- 2026-09-26 #546 -- the stages copy their state with `_state.copy_with` instead of `dataclasses.replace`, and every row moves DOWN. `dataclasses.replace` walks `fields()` and calls `__init__` on each copy, three frames on 3.11 and 3.12 and four from 3.13 where `copy_with` is one. One parse of the reference name makes 18 copies: six of the state, one each from tokenize, segment, classify, group, assign and post_rules, and twelve of single tokens, six each from classify and assign (`extract_delimited` returns the state unchanged when there is no delimiter, and `script_segment` returns early on ASCII input). That is the whole of the drop, 18 x 2 = 36 frames on 3.11 and 3.12 and 18 x 3 = 54 from 3.13 (recompute: wrap `copy_with` with a counter in the eight stage modules and parse the reference name). A field copy builds what `replace` builds for a dataclass that is decorated itself rather than inheriting the decoration, keeps the generated `__init__`, and has no `__post_init__` and no `init=False` field. `_copyable_fields` checks exactly those four, and `_COPY_FIELDS` runs it over `WorkToken`, `PendingAmbiguity` and `ParseState` at import, so a class that stops qualifying fails there and `copy_with` copies nothing else. `test_the_guard_refuses_a_class_a_field_copy_would_get_wrong` records, for each refused shape, what `replace` builds and what an unguarded copy would build instead. To mypy, `copy_with` is `from dataclasses import replace as copy_with`, which keeps the dataclass plugin's keyword and type checks at every call site; an assignment (`copy_with = dataclasses.replace`) would not, since the plugin keys on the callee's full name (measured: a misspelled field and a wrong-typed value both pass through the assignment and both fail through the import). Measured 2026-09-26 with `uv run python tools/perf/call_count.py --against e0f1a2f`, each row on its own interpreter, parse/facade: 3.11 406/443 → 370/407, 3.12 384/421 → 348/385, 3.13, 3.14 and 3.15 402/439 → 348/385. The rows drop by 40 and 58 rather than 36 and 54 because e0f1a2f already read 4 under every row, inside the band, and the new rows are set to what the harness reads now. `_LINK_BASELINE`'s 64-link clause reads 2587 → 2301 on 3.11. By stage (`--stages`, py3.11, ms per 1000 parses of the reference name): group 19.6 → 17.3, classify 13.4 → 9.8, assign 11.1 → 8.0, tokenize 8.1 → 6.6, post_rules 7.9 → 6.5, segment 2.6 → 1.7. BEHAVIOR IDENTICAL: the differential gate's report at all five baselines matches e0f1a2f's line for line apart from the path header. ### removed-v1-surface diff --git a/docs/release_log.rst b/docs/release_log.rst index 169a082e..d99d3cec 100644 --- a/docs/release_log.rst +++ b/docs/release_log.rst @@ -36,7 +36,7 @@ Release Log - **Change case repair to write an unlisted dotted credential and a roman numeral past iv in capitals.** ``HumanName("john smith x.y.z.").capitalize()`` gives ``John Smith X.Y.Z.`` where every release gave ``John Smith X.y.z.``, the dotted word being a suffix now (the ``unlisted_dotted_suffixes`` change above) and repaired as a listed acronym is; and ``john smith vi`` gives ``John Smith VI`` where every release gave ``John Smith Vi``, with ``vii``, ``viii`` and ``ix`` alike. Both are keyed on the suffix role: ``Jack X.Y.Z.``, which keeps its surname, still repairs as a name word (``Jack X.y.z.`` under ``force=True``), and ``john smith xi`` still gives ``John Smith Xi``, the parser reading ``xi`` as the surname. An unlisted dotted credential written in mixed case is kept as written on the default path, by the suffix change above (``john smith B.Tech.`` gives ``John Smith B.Tech.``), and reads all capitals under ``force=True`` (``John Smith B.TECH.``, where every release gave ``John Smith B.tech.``), which a ``capitalization_exceptions`` mask such as ``{"btech": "BTech"}`` undoes. Over the 1340 names in the differential corpora at the commit before this change (2026-09-23), 1 moves on the default path and 14 under ``force=True``. The recipe is the ``R4`` entry's 2026-09-23 MEASURED bullet in ``docs/design/decisions.md`` (#459) - - **Change the parse pipeline to copy its state without dataclasses.replace, about 10% faster per name.** Every stage returns a copy of its frozen state, and several copy tokens one at a time; ``dataclasses.replace`` goes through ``fields()`` and ``__init__`` on every one of those copies. The stages now copy fields directly through a small helper that refuses any class where that would differ from ``replace``. On py3.11 ``parse`` drops from 406 to 370 calls per parse of the benchmark's reference name and ``HumanName`` from 443 to 407, and the call-count baselines move with them. Recomputable with ``uv run python tools/perf/call_count.py --against e0f1a2f``; the counts for every interpreter are in the ``parse-cost`` entry of ``docs/design/decisions.md``. No user-visible behavior changes (#546) + - **Change the parse pipeline to copy its state without dataclasses.replace.** Every stage returns a copy of its frozen state, and several also copy tokens one at a time; ``dataclasses.replace`` goes through ``fields()`` and ``__init__`` on every one of those copies. The stages now copy fields directly through a small helper that is limited to the pipeline's own three dataclasses and checks them at import. One parse of the benchmark's reference name makes 36 fewer calls on py3.11 and 3.12 and 54 fewer from 3.13 (on 3.11, ``parse`` 406 to 370 and ``HumanName`` 443 to 407), and the call-count baselines move with them. Recomputable with ``uv run python tools/perf/call_count.py --against e0f1a2f``; the counts for every interpreter are in the ``parse-cost`` entry of ``docs/design/decisions.md``. No user-visible behavior changes (#546) **Additions** diff --git a/nameparser/_pipeline/_assign.py b/nameparser/_pipeline/_assign.py index 1e7663b1..a9aacd4d 100644 --- a/nameparser/_pipeline/_assign.py +++ b/nameparser/_pipeline/_assign.py @@ -1024,5 +1024,5 @@ def reads_as_a_suffix(m: int, titled: tuple[int, ...]) -> bool: for piece in state.pieces[seg_idx]: _set_roles(tokens, piece, Role.SUFFIX) return copy_with(state, tokens=tuple(tokens), - order=order, - ambiguities=tuple(ambiguities)) + order=order, + ambiguities=tuple(ambiguities)) diff --git a/nameparser/_pipeline/_classify.py b/nameparser/_pipeline/_classify.py index a94e3256..8b02070c 100644 --- a/nameparser/_pipeline/_classify.py +++ b/nameparser/_pipeline/_classify.py @@ -324,5 +324,5 @@ def classify(state: ParseState) -> ParseState: # The write rides the replace this stage already makes, so # recording the fact costs no frame of its own. return copy_with(state, tokens=tokens, - ambiguities=tuple(ambiguities), - one_case=one_case) + ambiguities=tuple(ambiguities), + one_case=one_case) diff --git a/nameparser/_pipeline/_post_rules.py b/nameparser/_pipeline/_post_rules.py index fab443b2..ec030523 100644 --- a/nameparser/_pipeline/_post_rules.py +++ b/nameparser/_pipeline/_post_rules.py @@ -887,4 +887,4 @@ def post_rules(state: ParseState) -> ParseState: tags=tokens[i].tags | {UNJOINED_CONJUNCTION_TAG}) _mark_suffix_entries(tokens, state) return copy_with(state, tokens=tuple(tokens), - ambiguities=tuple(ambiguities)) + ambiguities=tuple(ambiguities)) diff --git a/nameparser/_pipeline/_script_segment.py b/nameparser/_pipeline/_script_segment.py index bc94f714..28e83be2 100644 --- a/nameparser/_pipeline/_script_segment.py +++ b/nameparser/_pipeline/_script_segment.py @@ -179,7 +179,7 @@ def _split(state: ParseState, i: int, splits: tuple[int, ...], AmbiguityKind.SEGMENTATION, detail, tuple(range(i, i + added + 1))),) return copy_with(state, tokens=tokens, segments=segments, - ambiguities=ambiguities) + ambiguities=ambiguities) @functools.lru_cache(maxsize=16) diff --git a/nameparser/_pipeline/_segment.py b/nameparser/_pipeline/_segment.py index 98663431..fbce8749 100644 --- a/nameparser/_pipeline/_segment.py +++ b/nameparser/_pipeline/_segment.py @@ -37,7 +37,6 @@ """ from __future__ import annotations - from nameparser._pipeline._pieces import own_words from nameparser._pipeline._state import ( ParseState, PendingAmbiguity, Structure, comma_bucket, copy_with, @@ -56,10 +55,10 @@ def segment(state: ParseState) -> ParseState: main = [i for i, t in enumerate(state.tokens) if t.role is None] if not main: return copy_with(state, segments=(), - structure=Structure.NO_COMMA) + structure=Structure.NO_COMMA) if not state.comma_offsets: return copy_with(state, segments=(tuple(main),), - structure=Structure.NO_COMMA) + structure=Structure.NO_COMMA) buckets: list[list[int]] = [[] for _ in range(len(state.comma_offsets) + 1)] for i in main: # _state.comma_bucket, not a local bisect: classify asks the @@ -79,7 +78,7 @@ def segment(state: ParseState) -> ParseState: if len(groups) <= 1: segs = tuple(groups) if groups and groups[0] else (tuple(main),) return copy_with(state, segments=segs, - structure=Structure.NO_COMMA) + structure=Structure.NO_COMMA) # The case fact, asked LAZILY: only a comma form can turn on it # here, and only where the part after the first comma is a single @@ -306,6 +305,6 @@ def class_run(seg: tuple[int, ...]) -> bool: f"structures; consumed as suffix best-effort", tuple(seg))) return copy_with(state, segments=tuple(groups), - structure=structure, - ambiguities=tuple(ambiguities), - one_case=one_case) + structure=structure, + ambiguities=tuple(ambiguities), + one_case=one_case) diff --git a/nameparser/_pipeline/_state.py b/nameparser/_pipeline/_state.py index f11cb2db..9adef4bb 100644 --- a/nameparser/_pipeline/_state.py +++ b/nameparser/_pipeline/_state.py @@ -12,10 +12,11 @@ import bisect import dataclasses -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from dataclasses import dataclass from enum import Enum, auto -from typing import TypeVar +from types import MappingProxyType +from typing import TYPE_CHECKING, TypeVar from nameparser._lexicon import Lexicon from nameparser._policy import Policy @@ -23,47 +24,6 @@ Role, Segmenter, Span) -_T = TypeVar("_T") - -#: Field names per class, recorded the first time `copy_with` copies one. -_COPY_FIELDS: dict[type, tuple[str, ...]] = {} - - -def copy_with(obj: _T, /, **changes: object) -> _T: - """`dataclasses.replace` for the pipeline's own dataclasses, without - its per-call cost. - - Every stage returns a copy of the state, and several copy tokens one - at a time, so this runs many times per parse. The stdlib replace - walks `fields()` and goes through `__init__` on every call; for a - class whose `__init__` only assigns its fields, copying the fields - directly gives the same object. `_copy_fields` checks that once per - class and refuses any class where it would not hold - (decisions.md#parse-cost has the measurement). - """ - cls = type(obj) - names = _COPY_FIELDS.get(cls) or _copy_fields(cls) - new = object.__new__(cls) - for name in names: - value = changes.pop(name) if name in changes else getattr(obj, name) - object.__setattr__(new, name, value) - if changes: - raise TypeError( - f"{cls.__name__} has no field named {', '.join(sorted(changes))}") - return new - - -def _copy_fields(cls: type) -> tuple[str, ...]: - params = getattr(cls, "__dataclass_params__", None) - if (params is None or not params.init or hasattr(cls, "__post_init__") - or not all(f.init for f in dataclasses.fields(cls))): - raise TypeError( - f"copy_with cannot copy {cls.__name__}: it needs a dataclass " - "whose generated __init__ only assigns its fields") - names = _COPY_FIELDS[cls] = tuple(f.name for f in dataclasses.fields(cls)) - return names - - # The comma characters (ASCII/Arabic/fullwidth, #265). Shared here so # tokenize (separators/segmentation) and extract (close-quote # boundaries) cannot drift apart. @@ -253,3 +213,57 @@ class ParseState: #: lean) rather than asking again. one_case: bool | None = None ambiguities: tuple[PendingAmbiguity, ...] = () + + +def _copyable_fields(cls: type) -> tuple[str, ...]: + """The fields `copy_with` carries for `cls`, or TypeError where a + field copy would not build what `dataclasses.replace` builds: the + class must be decorated itself (not inherit the decoration) and keep + the generated `__init__`, with no `__post_init__` and no + `init=False` field.""" + params = cls.__dict__.get("__dataclass_params__") + init = cls.__dict__.get("__init__") + # dataclasses compiles the __init__ it generates from a string; one + # written in the class body carries its source file instead. + generated = init is not None and init.__code__.co_filename == "" + if (params is None or not generated + or hasattr(cls, "__post_init__") + or not all(f.init for f in dataclasses.fields(cls))): + raise TypeError( + f"copy_with cannot copy {cls.__name__}: it needs a dataclass " + "whose generated __init__ only assigns its fields") + return tuple(f.name for f in dataclasses.fields(cls)) + + +#: Built once, at import: a pipeline class that stops qualifying fails +#: here rather than in a parse, and `copy_with` copies nothing else. +_COPY_FIELDS: Mapping[type, tuple[str, ...]] = MappingProxyType({ + cls: _copyable_fields(cls) + for cls in (WorkToken, PendingAmbiguity, ParseState)}) + +_T = TypeVar("_T") + +if TYPE_CHECKING: + # mypy's dataclass plugin checks replace's keywords against the + # class, which a `**changes: object` signature would not. + from dataclasses import replace as copy_with +else: + def copy_with(obj: _T, /, **changes: object) -> _T: + """`dataclasses.replace` for the pipeline's own dataclasses, + without its per-call cost: a direct field copy, which for the + classes in `_COPY_FIELDS` builds the same object + (decisions.md#parse-cost has the measurement).""" + cls = type(obj) + names = _COPY_FIELDS.get(cls) + if names is None: + raise TypeError( + f"copy_with copies only the pipeline's own dataclasses, " + f"not {cls.__name__}") + new = object.__new__(cls) + for name in names: + value = changes.pop(name) if name in changes else getattr(obj, name) + object.__setattr__(new, name, value) + if changes: + raise TypeError(f"{cls.__name__} has no field named " + f"{', '.join(sorted(changes))}") + return new diff --git a/nameparser/_pipeline/_tokenize.py b/nameparser/_pipeline/_tokenize.py index fd8ce3b1..300149d4 100644 --- a/nameparser/_pipeline/_tokenize.py +++ b/nameparser/_pipeline/_tokenize.py @@ -191,6 +191,6 @@ def _containing(offset: int) -> tuple[int, ...]: else copy_with(a, indices=_containing(a.origin)) for a in ambiguities) return copy_with(state, tokens=tuple(tokens), - comma_offsets=tuple(sorted(commas)), - interpunct_offsets=tuple(sorted(interpuncts)), - ambiguities=ambiguities) + comma_offsets=tuple(sorted(commas)), + interpunct_offsets=tuple(sorted(interpuncts)), + ambiguities=ambiguities) diff --git a/tests/v2/pipeline/test_state.py b/tests/v2/pipeline/test_state.py index dbfe05bf..40a58582 100644 --- a/tests/v2/pipeline/test_state.py +++ b/tests/v2/pipeline/test_state.py @@ -1,10 +1,12 @@ import dataclasses +from collections.abc import Callable import pytest from nameparser._lexicon import Lexicon from nameparser._pipeline._state import ( - ParseState, PendingAmbiguity, Structure, WorkToken, copy_with, + ParseState, PendingAmbiguity, Structure, WorkToken, _copyable_fields, + copy_with, ) from nameparser._policy import Policy from nameparser._types import AmbiguityKind, Role, Span @@ -49,7 +51,12 @@ def test_copy_with_builds_what_dataclasses_replace_builds() -> None: def test_copy_with_rejects_a_field_the_class_does_not_have() -> None: with pytest.raises(TypeError, match="no field named rol"): - copy_with(WorkToken("x", Span(0, 1)), rol=Role.GIVEN) + copy_with(WorkToken("x", Span(0, 1)), rol=Role.GIVEN) # type: ignore[call-arg] + + +def test_copy_with_copies_only_the_pipeline_dataclasses() -> None: + with pytest.raises(TypeError, match="not _Validated"): + copy_with(_Validated(1), value=2) @dataclasses.dataclass(frozen=True) @@ -61,19 +68,82 @@ def __post_init__(self) -> None: raise ValueError("negative") +@dataclasses.dataclass(frozen=True) +class _OwnInit: + value: int + + def __init__(self, value: int) -> None: + if value < 0: + raise ValueError("negative") + object.__setattr__(self, "value", value) + + @dataclasses.dataclass(frozen=True) class _Derived: value: int doubled: int = dataclasses.field(init=False, default=0) -@pytest.mark.parametrize("obj", [_Validated(1), _Derived(1)]) -def test_copy_with_refuses_a_class_whose_init_does_more_than_assign( - obj: object) -> None: - # Copying fields would skip the validation or recompute the - # init=False field that dataclasses.replace goes through __init__ for. +@dataclasses.dataclass +class _Base: + value: int + + +class _Undecorated(_Base): + def __init__(self, value: int) -> None: + super().__init__(value) + self.extra = "set by __init__" + + +def _derived_with_doubled_set() -> _Derived: + obj = _Derived(1) + object.__setattr__(obj, "doubled", 2) + return obj + + +def _unguarded_copy(obj: object, **changes: object) -> object: + new = object.__new__(type(obj)) + for f in dataclasses.fields(obj): # type: ignore[arg-type] + object.__setattr__(new, f.name, changes.get(f.name, getattr(obj, f.name))) + return new + + +def _outcome(copy: Callable[[], object]) -> object: + """What a caller reads off the copy: every field, plus any attribute + set outside the fields, or the error the copy raised.""" + try: + result = copy() + except ValueError as exc: + return type(exc).__name__ + names = [f.name for f in dataclasses.fields(result)] # type: ignore[arg-type] + return {name: getattr(result, name) for name in dict.fromkeys([*names, *vars(result)])} + + +#: What dataclasses.replace builds for each class the guard refuses, and +#: what a field copy without the guard would build instead: the negative +#: control, recorded so the refusal test cannot pass vacuously. +_UNGUARDED_EFFECT = [ + ("validating __post_init__", _Validated(1), {"value": -1}, + "ValueError", {"value": -1}), + ("validating __init__", _OwnInit(1), {"value": -1}, + "ValueError", {"value": -1}), + ("init=False field", _derived_with_doubled_set(), {"value": 3}, + {"value": 3, "doubled": 0}, {"value": 3, "doubled": 2}), + ("undecorated subclass", _Undecorated(1), {"value": 2}, + {"value": 2, "extra": "set by __init__"}, {"value": 2}), +] + + +@pytest.mark.parametrize( + "shape,obj,changes,by_replace,by_field_copy", _UNGUARDED_EFFECT, + ids=[row[0] for row in _UNGUARDED_EFFECT]) +def test_the_guard_refuses_a_class_a_field_copy_would_get_wrong( + shape: str, obj: object, changes: dict[str, object], + by_replace: object, by_field_copy: object) -> None: + assert _outcome(lambda: dataclasses.replace(obj, **changes)) == by_replace # type: ignore[type-var] + assert _outcome(lambda: _unguarded_copy(obj, **changes)) == by_field_copy with pytest.raises(TypeError, match="cannot copy"): - copy_with(obj, value=2) + _copyable_fields(type(obj)) def test_worktoken_carries_optional_role() -> None: