From 37d5c6915cbf0672581f49989f17b875329703dd Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 01:53:34 -0700 Subject: [PATCH 1/2] Simplify pass over the v1.2.1..HEAD changes Behavior-preserving cleanups from a four-angle review (reuse, simplification, efficiency, altitude) of everything since v1.2.1: - join_on_conjunctions: share one register_joined_piece helper for the title/prefix registration duplicated across both join branches - Consolidate the lenient post-comma suffix rule into is_suffix_lenient(); are_suffixes_after_comma uses it, and the single-use is_suffix_at_lastname_comma_end method is inlined at its only call site - cap_word: compute the exception-lookup key once instead of up to four lc()/replace() calls per word - Constants.__setstate__: drop the verification loop that could never fire for state produced by __getstate__ - Extract _is_dunder() for the guard copy-pasted across four TupleManager/RegexTupleManager attribute hooks - tests: add FlaggedConstantsTestBase replacing seven copy-pasted setup_method/hn() fixture pairs; drop the two invariant tests that duplicate prefixes.py's import-time asserts Co-Authored-By: Claude Fable 5 --- AGENTS.md | 2 +- nameparser/config/__init__.py | 61 +++++++------- nameparser/parser.py | 94 +++++++++------------- tests/base.py | 20 ++++- tests/test_east_slavic_patronymic_order.py | 18 ++--- tests/test_middle_name_as_last.py | 18 ++--- tests/test_prefixes.py | 14 +--- tests/test_suffixes.py | 3 +- tests/test_turkic_patronymic_order.py | 26 ++---- 9 files changed, 111 insertions(+), 145 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 97e5bb2..8f5b865 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,7 +134,7 @@ Add a dedicated `copy.deepcopy()` round-trip test for it too (see `test_regexes_ **Cyrillic suffix regexes need `re.I` even when the pattern is suffix-only** — a Latin title-cased word (`Ivanovich`) keeps its suffix lowercase, so `re.I` seemed skippable; but an irregular Cyrillic suffix can be nearly the whole word (`ильич`), so title-casing capitalizes into the suffix itself (`Ильич`). `east_slavic_patronymic_cyrillic` shipped without `re.I` on the Latin reasoning and silently failed on capitalized irregular forms — don't assume Latin's title-case safety transfers to Cyrillic. (#185) -**`suffix_not_acronyms` vs `is_an_initial` tension** — single-letter roman numeral suffixes (`i`, `v`) are in `suffix_not_acronyms` but also match the `is_an_initial` regex (single uppercase letter), so `is_suffix()` rejects them. Two separate code paths need context-aware workarounds: (1) suffix-comma detection uses `are_suffixes_after_comma()` which bypasses `is_suffix()` for `suffix_not_acronyms` members; (2) lastname-comma post-comma parsing uses `is_suffix_at_lastname_comma_end()` which only fires when `nxt is None` and `len(parts)==2` (no `parts[2]` suffix segment). See issues #136, #144. +**`suffix_not_acronyms` vs `is_an_initial` tension** — single-letter roman numeral suffixes (`i`, `v`) are in `suffix_not_acronyms` but also match the `is_an_initial` regex (single uppercase letter), so `is_suffix()` rejects them. The lenient test lives in `is_suffix_lenient()`, which accepts `suffix_not_acronyms` members unconditionally and is only safe in unambiguous positions: (1) suffix-comma detection uses it via `are_suffixes_after_comma()`; (2) lastname-comma post-comma parsing uses it inline, only when `nxt is None` and `len(parts)==2` (no `parts[2]` suffix segment). See issues #136, #144. **Expected-failure tests use `@pytest.mark.xfail`** — the conftest parametrized fixture breaks `@unittest.expectedFailure`; always use `@pytest.mark.xfail` instead. diff --git a/nameparser/config/__init__.py b/nameparser/config/__init__.py index 0608d07..a66b19b 100644 --- a/nameparser/config/__init__.py +++ b/nameparser/config/__init__.py @@ -140,40 +140,47 @@ def clear(self) -> Self: T = TypeVar('T') +def _is_dunder(attr: str) -> bool: + # Dunder names are Python's protocol probes (copy looks up __deepcopy__, + # inspect.unwrap looks up __wrapped__, typing's GenericAlias.__call__ sets + # __orig_class__, ...), never config keys. The TupleManager attribute hooks + # all route dunders to normal object-attribute behavior so those probes + # work instead of being mistaken for dict entries. + return attr.startswith("__") and attr.endswith("__") + + class TupleManager(dict[str, T]): ''' - A dictionary with dot.notation access. Subclass of ``dict``. Makes the tuple constants + A dictionary with dot.notation access. Subclass of ``dict``. Makes the tuple constants more friendly. ''' def __getattr__(self, attr: str) -> T | None: - # Dunder names are Python's protocol probes (copy looks up __deepcopy__, - # inspect.unwrap looks up __wrapped__, ...), never config keys. Report - # them as genuinely absent so hasattr() is honest and those probes work; - # otherwise the dict default is mistaken for a real protocol hook. See - # RegexTupleManager.__getattr__ for the concrete failure this prevents. - if attr.startswith("__") and attr.endswith("__"): + # Report dunders as genuinely absent so hasattr() is honest and + # protocol probes work; otherwise the dict default is mistaken for a + # real protocol hook. See RegexTupleManager.__getattr__ for the + # concrete failure this prevents. + if _is_dunder(attr): raise AttributeError(attr) return self.get(attr) def __setattr__(self, attr: str, value: T) -> None: - # Dunder names are Python's protocol probes, not config keys -- same - # rationale as __getattr__ above. Concretely: constructing a - # subscripted generic, e.g. TupleManager[re.Pattern[str] | str](...), - # makes typing's GenericAlias.__call__ set `__orig_class__` on the new - # instance right after __init__ returns. Without this guard that - # assignment falls through to dict.__setitem__ and silently inserts a - # bogus '__orig_class__' entry into the dict itself, corrupting - # .values()/iteration. Fall back to normal object attribute storage - # for dunders; everything else keeps the dict-backed dot-notation - # behavior this class exists for. - if attr.startswith("__") and attr.endswith("__"): + # Fall back to normal object attribute storage for dunders; everything + # else keeps the dict-backed dot-notation behavior this class exists + # for. Concretely: constructing a subscripted generic, e.g. + # TupleManager[re.Pattern[str] | str](...), makes typing's + # GenericAlias.__call__ set `__orig_class__` on the new instance right + # after __init__ returns. Without this guard that assignment falls + # through to dict.__setitem__ and silently inserts a bogus + # '__orig_class__' entry into the dict itself, corrupting + # .values()/iteration. + if _is_dunder(attr): object.__setattr__(self, attr, value) else: self[attr] = value def __delattr__(self, attr: str) -> None: - if attr.startswith("__") and attr.endswith("__"): + if _is_dunder(attr): object.__delattr__(self, attr) else: del self[attr] @@ -194,12 +201,10 @@ def __reduce__(self) -> tuple[type, tuple[()], Mapping[str, T]]: class RegexTupleManager(TupleManager[re.Pattern[str]]): def __getattr__(self, attr: str) -> re.Pattern[str]: - # Dunder names are Python's protocol probes (copy.deepcopy looks up - # __deepcopy__, inspect.unwrap looks up __wrapped__, ...), never regex - # keys. Report them as genuinely absent; otherwise the EMPTY_REGEX + # Report dunders as genuinely absent; otherwise the EMPTY_REGEX # default is mistaken for a real protocol hook — e.g. copy.deepcopy # tries to call the returned re.Pattern and raises TypeError. - if attr.startswith("__") and attr.endswith("__"): + if _is_dunder(attr): raise AttributeError(attr) return self.get(attr, EMPTY_REGEX) @@ -536,16 +541,6 @@ def __setstate__(self, state: Mapping[str, Any]) -> None: self._pst = None for name, value in state.items(): setattr(self, name, value) - # Verify each descriptor-backed attr was restored. Without this, a missing - # key surfaces later as AttributeError: 'Constants' object has no attribute - # '_prefixes' — the private mangled name, not the public one, making it - # very hard to diagnose. - for attr in (n for n, v in vars(type(self)).items() if isinstance(v, _CachedUnionMember)): - if not hasattr(self, '_' + attr): - raise ValueError( - f"Pickle state is missing required field {attr!r}. " - "The state blob may be truncated or from an incompatible version." - ) def __getstate__(self) -> Mapping[str, Any]: # Pickle the instance's own configuration: the collections built in diff --git a/nameparser/parser.py b/nameparser/parser.py index 39e61e7..f170108 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -663,43 +663,24 @@ def are_suffixes(self, pieces: Iterable[str]) -> bool: return False return True - def are_suffixes_after_comma(self, pieces: Iterable[str]) -> bool: - """Like are_suffixes, but pieces found in suffix_not_acronyms are - accepted unconditionally without passing through is_suffix(). + def is_suffix_lenient(self, piece: str) -> bool: + """Like is_suffix(), but suffix_not_acronyms members are accepted + unconditionally, bypassing is_suffix()'s is_an_initial() veto. - Used when detecting suffix-comma format (e.g. "John Ingram, V") where - the post-comma position is unambiguous. This covers all - suffix_not_acronyms members (i, ii, iii, iv, v, jr, sr, etc.), - case-insensitively, including single-letter entries that is_suffix() - would otherwise reject via is_an_initial(). + This covers all suffix_not_acronyms members (i, ii, iii, iv, v, jr, + sr, etc.), case-insensitively, including single-letter entries that + is_suffix() would otherwise reject. Only safe for pieces in + unambiguous positions, e.g. after a comma ("John Ingram, V"). """ - for piece in pieces: - if lc(piece) in self.C.suffix_not_acronyms: - continue - if not self.is_suffix(piece): - return False - return True - - def is_suffix_at_lastname_comma_end(self, piece: str, nxt: str | None, parts: list[str]) -> bool: - """True when ``piece`` is a suffix_not_acronyms member that should be - treated as a suffix at the end of ``parts[1]`` (the post-comma segment) - in a lastname-comma name, where ``parts`` is the full comma-split of the - name string. - - Returns True only when all three conditions hold: - - ``nxt is None``: piece is the last token in the post-comma segment - - ``len(parts) == 2``: no ``parts[2]`` suffix segment exists - - ``lc(piece) in suffix_not_acronyms`` + return lc(piece) in self.C.suffix_not_acronyms or self.is_suffix(piece) - When ``parts[2]`` exists the caller already declared an explicit suffix - via comma (e.g. 'Doe, Rev. John V, Jr.'), making the trailing token more - likely a middle initial; ``len(parts) == 2`` excludes that case. - Used as an OR alternative to ``is_suffix()`` for pieces that - ``is_suffix()`` would reject via ``is_an_initial()``. + def are_suffixes_after_comma(self, pieces: Iterable[str]) -> bool: + """Return True if all pieces are suffixes by the lenient + :py:func:`is_suffix_lenient` test. Used when detecting suffix-comma + format (e.g. "John Ingram, V") where the post-comma position is + unambiguous. """ - return (nxt is None - and len(parts) == 2 - and lc(piece) in self.C.suffix_not_acronyms) + return all(self.is_suffix_lenient(piece) for piece in pieces) def is_rootname(self, piece: str) -> bool: """ @@ -1147,7 +1128,16 @@ def parse_full_name(self) -> None: if not self.first: self.first_list.append(piece) continue - if self.is_suffix(piece) or self.is_suffix_at_lastname_comma_end(piece, nxt, parts): + # A trailing token in a two-part lastname-comma name is + # unambiguously positioned, so use the lenient test that + # accepts suffix_not_acronyms members is_suffix() would + # veto as initials. When parts[2] exists the caller + # already declared an explicit suffix via comma (e.g. + # 'Doe, Rev. John V, Jr.'), making the trailing token + # more likely a middle initial. + if self.is_suffix(piece) or \ + (nxt is None and len(parts) == 2 + and self.is_suffix_lenient(piece)): self.suffix_list.append(piece) continue self.middle_list.append(piece) @@ -1266,6 +1256,16 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = # refresh conjunction index locations conj_index = [i for i, piece in enumerate(pieces) if self.is_conjunction(piece)] + def register_joined_piece(new_piece: str, neighbor: str) -> None: + if self.is_title(neighbor): + # when joining to a title, make new_piece a title too + self.C.titles.add(new_piece) + if self.is_prefix(neighbor): + # when joining to a prefix, make new_piece a prefix too, so + # e.g. "von" + "und" bridges into "von und" and can still + # chain onto a following prefix/lastname (see "von und zu") + self.C.prefixes.add(new_piece) + for i in conj_index: if len(pieces[i]) == 1 and total_length < 4 and pieces[i].isalpha(): # if there are only 3 total parts (minus known titles, suffixes @@ -1276,14 +1276,7 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = if i == 0: new_piece = " ".join(pieces[i:i+2]) - if self.is_title(pieces[i+1]): - # when joining to a title, make new_piece a title too - self.C.titles.add(new_piece) - if self.is_prefix(pieces[i+1]): - # when joining to a prefix, make new_piece a prefix too, so - # e.g. "von" + "und" bridges into "von und" and can still - # chain onto a following prefix/lastname (see "von und zu") - self.C.prefixes.add(new_piece) + register_joined_piece(new_piece, pieces[i+1]) pieces[i] = new_piece pieces.pop(i+1) # subtract 1 from the index of all the remaining conjunctions @@ -1293,14 +1286,7 @@ def join_on_conjunctions(self, pieces: list[str], additional_parts_count: int = else: new_piece = " ".join(pieces[i-1:i+2]) - if self.is_title(pieces[i-1]): - # when joining to a title, make new_piece a title too - self.C.titles.add(new_piece) - if self.is_prefix(pieces[i-1]): - # when joining to a prefix, make new_piece a prefix too, so - # e.g. "von" + "und" bridges into "von und" and can still - # chain onto a following prefix/lastname (see "von und zu") - self.C.prefixes.add(new_piece) + register_joined_piece(new_piece, pieces[i-1]) pieces[i-1] = new_piece pieces.pop(i) rm_count = 2 @@ -1372,10 +1358,10 @@ def cap_word(self, word: str, attribute: HumanNameAttributeT) -> str: or self.is_conjunction(word): return word.lower() exceptions = self.C.capitalization_exceptions - if lc(word) in exceptions: - return exceptions[lc(word)] - if lc(word).replace('.', '') in exceptions: - return exceptions[lc(word).replace('.', '')] + key = lc(word) + for k in (key, key.replace('.', '')): + if k in exceptions: + return exceptions[k] mac_match = self.C.regexes.mac.match(word) if mac_match: def cap_after_mac(m: re.Match) -> str: diff --git a/tests/base.py b/tests/base.py index 444c841..b563fb3 100644 --- a/tests/base.py +++ b/tests/base.py @@ -1,6 +1,7 @@ -from typing import Generic, TypeVar +from typing import Any, ClassVar, Generic, TypeVar from nameparser import HumanName +from nameparser.config import Constants T = TypeVar('T') @@ -51,3 +52,20 @@ def assertIsNone(self, expr: object, msg: object = None) -> None: def assertIsNotNone(self, expr: object, msg: object = None) -> None: assert expr is not None, msg or "unexpectedly None" + + +class FlaggedConstantsTestBase(HumanNameTestBase[T]): + """Base for test classes that parse with a dedicated, flagged Constants. + + Subclasses set ``constants_kwargs``; each test method gets a fresh + ``Constants(**constants_kwargs)`` via ``setup_method``, and ``hn()`` + parses with it. + """ + + constants_kwargs: ClassVar[dict[str, Any]] = {} + + def setup_method(self) -> None: + self.C = Constants(**self.constants_kwargs) + + def hn(self, name: str) -> HumanName: + return HumanName(name, constants=self.C) diff --git a/tests/test_east_slavic_patronymic_order.py b/tests/test_east_slavic_patronymic_order.py index 3d4015c..314592a 100644 --- a/tests/test_east_slavic_patronymic_order.py +++ b/tests/test_east_slavic_patronymic_order.py @@ -1,6 +1,6 @@ from nameparser import HumanName from nameparser.config import Constants -from tests.base import HumanNameTestBase +from tests.base import FlaggedConstantsTestBase, HumanNameTestBase def test_latin_patronymic_matches() -> None: @@ -49,14 +49,10 @@ def test_cyrillic_patronymic_rejects_non_patronymic() -> None: assert not C.regexes.east_slavic_patronymic_cyrillic.search("Иванов") -class PatronymicNameOrderReorderTests(HumanNameTestBase): +class PatronymicNameOrderReorderTests(FlaggedConstantsTestBase): """Names that SHOULD be rotated when the flag is on.""" - def setup_method(self) -> None: - self.C = Constants(patronymic_name_order=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"patronymic_name_order": True} def test_canonical_latin(self) -> None: n = self.hn("Ivanov Ivan Ivanovich") @@ -121,14 +117,10 @@ def test_western_patronymic_surname_reordered_when_flag_on(self) -> None: assert n.last == "David" -class PatronymicNameOrderGuardsTests(HumanNameTestBase): +class PatronymicNameOrderGuardsTests(FlaggedConstantsTestBase): """Names that must NOT be reordered even when the flag is on.""" - def setup_method(self) -> None: - self.C = Constants(patronymic_name_order=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"patronymic_name_order": True} def test_already_correct_order(self) -> None: # middle is patronymic → already in Western order, do not rotate diff --git a/tests/test_middle_name_as_last.py b/tests/test_middle_name_as_last.py index bfec02b..77cba11 100644 --- a/tests/test_middle_name_as_last.py +++ b/tests/test_middle_name_as_last.py @@ -1,6 +1,6 @@ from nameparser import HumanName from nameparser.config import Constants -from tests.base import HumanNameTestBase +from tests.base import FlaggedConstantsTestBase, HumanNameTestBase class MiddleNameAsLastFlagTests(HumanNameTestBase): @@ -20,13 +20,9 @@ def test_does_not_affect_other_instance(self) -> None: assert C2.middle_name_as_last is False -class MiddleNameAsLastFoldTests(HumanNameTestBase): +class MiddleNameAsLastFoldTests(FlaggedConstantsTestBase): - def setup_method(self) -> None: - self.C = Constants(middle_name_as_last=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"middle_name_as_last": True} def test_fold_no_comma(self) -> None: n = self.hn("Mohamad Ahmad Ali Hassan") @@ -98,17 +94,13 @@ def test_default_constants_unaffected(self) -> None: self.m(n.last, "Hassan", n) -class MiddleNameAsLastWithPatronymicOrderTests(HumanNameTestBase): +class MiddleNameAsLastWithPatronymicOrderTests(FlaggedConstantsTestBase): """Both localization flags on: patronymic reordering must settle first/middle/last before the fold collapses middle into last, per the design's stated ordering rationale (post_process() runs the patronymic hook before the middle_name_as_last hook).""" - def setup_method(self) -> None: - self.C = Constants(middle_name_as_last=True, patronymic_name_order=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"middle_name_as_last": True, "patronymic_name_order": True} def test_rotate_then_fold_no_comma(self) -> None: # patronymic_name_order rotates "Ivanov Petr Sergeyevich" to diff --git a/tests/test_prefixes.py b/tests/test_prefixes.py index 25a74d9..216ad49 100644 --- a/tests/test_prefixes.py +++ b/tests/test_prefixes.py @@ -2,8 +2,7 @@ from nameparser import HumanName from nameparser.config import CONSTANTS, Constants -from nameparser.config.prefixes import PREFIXES, NON_FIRST_NAME_PREFIXES -from nameparser.config.bound_first_names import BOUND_FIRST_NAMES +from nameparser.config.prefixes import NON_FIRST_NAME_PREFIXES from tests.base import HumanNameTestBase @@ -166,14 +165,9 @@ def test_comma_three_conjunctions(self) -> None: self.m(hn.middle, "Q. Xavier", hn) self.m(hn.suffix, "III", hn) - def test_non_first_name_prefixes_subset_of_prefixes(self) -> None: - # Every non-first-name prefix must still be a prefix so it joins forward. - self.assertTrue(NON_FIRST_NAME_PREFIXES <= PREFIXES) - - def test_non_first_name_prefixes_disjoint_from_bound_first_names(self) -> None: - # A word cannot be both "joins to the first name" and "never a first - # name" (e.g. 'abu' is a bound_first_name, so it is excluded here). - self.assertEqual(NON_FIRST_NAME_PREFIXES & BOUND_FIRST_NAMES, set()) + # The subset-of-PREFIXES and disjoint-from-BOUND_FIRST_NAMES invariants + # are enforced by import-time asserts in nameparser/config/prefixes.py, + # so they are not repeated as tests here. def test_non_first_name_prefixes_expected_members(self) -> None: # 'abu' is in PREFIXES but excluded (it is a bound_first_name); diff --git a/tests/test_suffixes.py b/tests/test_suffixes.py index 59f73cb..8e7248e 100644 --- a/tests/test_suffixes.py +++ b/tests/test_suffixes.py @@ -204,7 +204,8 @@ def test_roman_numeral_i_after_single_initial_lastname_comma_format(self) -> Non def test_roman_numeral_i_with_explicit_suffix_comma_known_limitation(self) -> None: # When an explicit suffix comma is present (len(parts)==3), the trailing 'I' # is conservatively left in middle to avoid misclassifying true initials. - # This is a known limitation of is_suffix_at_lastname_comma_end (issue #144). + # This is a known limitation of the lastname-comma lenient-suffix + # guard in parse_full_name (issue #144). hn = HumanName("Maier, Amy I, Jr.") self.m(hn.suffix, "I, Jr.", hn) diff --git a/tests/test_turkic_patronymic_order.py b/tests/test_turkic_patronymic_order.py index 19c608e..9b02660 100644 --- a/tests/test_turkic_patronymic_order.py +++ b/tests/test_turkic_patronymic_order.py @@ -1,6 +1,6 @@ from nameparser import HumanName from nameparser.config import Constants -from tests.base import HumanNameTestBase +from tests.base import FlaggedConstantsTestBase, HumanNameTestBase def test_marker_is_whole_word_not_substring() -> None: @@ -15,14 +15,10 @@ def test_marker_is_whole_word_not_substring() -> None: assert not C.regexes.turkic_patronymic_marker_cyrillic.match("Оглуев") -class TurkicPatronymicNameOrderReorderTests(HumanNameTestBase): +class TurkicPatronymicNameOrderReorderTests(FlaggedConstantsTestBase): """Names that SHOULD be rotated when the flag is on.""" - def setup_method(self) -> None: - self.C = Constants(patronymic_name_order=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"patronymic_name_order": True} def test_oglu(self) -> None: n = self.hn("Aliyev Vusal Said oglu") @@ -195,14 +191,10 @@ def test_all_caps_cyrillic(self) -> None: assert n.last == "АЛИЕВ" -class TurkicPatronymicNameOrderGuardsTests(HumanNameTestBase): +class TurkicPatronymicNameOrderGuardsTests(FlaggedConstantsTestBase): """Names that must NOT be reordered even when the flag is on.""" - def setup_method(self) -> None: - self.C = Constants(patronymic_name_order=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"patronymic_name_order": True} def test_already_correct_natural_order(self) -> None: n = self.hn("Vusal Said oglu Aliyev") @@ -248,14 +240,10 @@ def test_reversed_order_unchanged(self) -> None: assert n.last == "oglu" -class PatronymicHandlerInteractionTests(HumanNameTestBase): +class PatronymicHandlerInteractionTests(FlaggedConstantsTestBase): """Both handlers run in sequence under the same flag; confirm no interference.""" - def setup_method(self) -> None: - self.C = Constants(patronymic_name_order=True) - - def hn(self, name: str) -> HumanName: - return HumanName(name, constants=self.C) + constants_kwargs = {"patronymic_name_order": True} def test_east_slavic_shape_unaffected_by_turkic_handler(self) -> None: # If handle_turkic_patronymic_name_order() ran BEFORE the East-Slavic From ba6e8e6a3e4e4ac09952e7bef8ed0d843abcda60 Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sat, 4 Jul 2026 02:06:02 -0700 Subject: [PATCH 2/2] Restore pickle-state verification loop; trim duplicated dunder comments Review of #204 flagged the __setstate__ verification loop drop as an undisclosed diagnostic regression (truncated/incompatible pickle state would surface as a confusing AttributeError instead of a clear ValueError), so it's restored. Also trims the two __getattr__ comments that duplicated _is_dunder()'s rationale instead of stating only the site-specific consequence. Co-Authored-By: Claude Sonnet 5 --- nameparser/config/__init__.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/nameparser/config/__init__.py b/nameparser/config/__init__.py index a66b19b..6c2a7ab 100644 --- a/nameparser/config/__init__.py +++ b/nameparser/config/__init__.py @@ -156,10 +156,7 @@ class TupleManager(dict[str, T]): ''' def __getattr__(self, attr: str) -> T | None: - # Report dunders as genuinely absent so hasattr() is honest and - # protocol probes work; otherwise the dict default is mistaken for a - # real protocol hook. See RegexTupleManager.__getattr__ for the - # concrete failure this prevents. + # Otherwise the dict default (None) is mistaken for a real protocol hook. if _is_dunder(attr): raise AttributeError(attr) return self.get(attr) @@ -201,9 +198,8 @@ def __reduce__(self) -> tuple[type, tuple[()], Mapping[str, T]]: class RegexTupleManager(TupleManager[re.Pattern[str]]): def __getattr__(self, attr: str) -> re.Pattern[str]: - # Report dunders as genuinely absent; otherwise the EMPTY_REGEX - # default is mistaken for a real protocol hook — e.g. copy.deepcopy - # tries to call the returned re.Pattern and raises TypeError. + # Otherwise EMPTY_REGEX is returned for a dunder probe; copy.deepcopy + # then tries to call the returned re.Pattern and raises TypeError. if _is_dunder(attr): raise AttributeError(attr) return self.get(attr, EMPTY_REGEX) @@ -541,6 +537,16 @@ def __setstate__(self, state: Mapping[str, Any]) -> None: self._pst = None for name, value in state.items(): setattr(self, name, value) + # Verify each descriptor-backed attr was restored. Without this, a missing + # key surfaces later as AttributeError: 'Constants' object has no attribute + # '_prefixes' — the private mangled name, not the public one, making it + # very hard to diagnose. + for attr in (n for n, v in vars(type(self)).items() if isinstance(v, _CachedUnionMember)): + if not hasattr(self, '_' + attr): + raise ValueError( + f"Pickle state is missing required field {attr!r}. " + "The state blob may be truncated or from an incompatible version." + ) def __getstate__(self) -> Mapping[str, Any]: # Pickle the instance's own configuration: the collections built in