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..6c2a7ab 100644 --- a/nameparser/config/__init__.py +++ b/nameparser/config/__init__.py @@ -140,40 +140,44 @@ 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("__"): + # Otherwise the dict default (None) is mistaken for a real protocol hook. + 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 +198,9 @@ 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 - # 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("__"): + # 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) 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