From 4097e7bcf89bde3b1781148a41c2a89c43b29ccc Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sun, 5 Jul 2026 15:36:15 -0700 Subject: [PATCH 1/2] Fix constants argument silently discarding Constants subclasses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit __init__ validated the constants argument with an exact-type check (type(self.C) is not type(CONSTANTS)), so an instance of a Constants subclass failed the check and was silently replaced with fresh defaults, discarding the caller's configuration without any error. Validate explicitly instead: None means a private per-instance Constants (unchanged), any Constants instance — including subclasses — is used as given, and anything else raises TypeError rather than being silently swapped for defaults. The signature annotation now admits None, which was always accepted and documented. Also fix stale prose in customize.rst: "pass something falsey" no longer describes the behavior (only None gets per-instance config now), and the keyword argument is `constants`, not `constant`. Closes #226 Co-Authored-By: Claude Fable 5 --- docs/customize.rst | 7 ++++--- docs/release_log.rst | 1 + nameparser/parser.py | 11 ++++++++--- tests/test_constants.py | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/docs/customize.rst b/docs/customize.rst index dab9ac3..d49ea8c 100644 --- a/docs/customize.rst +++ b/docs/customize.rst @@ -456,10 +456,11 @@ e.g. parsing names on multiple threads. If you'd prefer new instances to have their own config values, one shortcut is to pass -``None`` as the second argument (or ``constant`` keyword argument) when +``None`` as the second argument (or ``constants`` keyword argument) when instantiating ``HumanName``. Each instance always has a ``C`` attribute, but if -you didn't pass something falsey to the ``constants`` argument then it's a -reference to the module-level config values with the behavior described above. +you didn't pass ``None`` (or your own :py:class:`~nameparser.config.Constants` +instance) to the ``constants`` argument then it's a reference to the +module-level config values with the behavior described above. .. doctest:: module config :options: +ELLIPSIS, +NORMALIZE_WHITESPACE diff --git a/docs/release_log.rst b/docs/release_log.rst index e405fa6..5391ed9 100644 --- a/docs/release_log.rst +++ b/docs/release_log.rst @@ -1,6 +1,7 @@ Release Log =========== * 1.3.0 - Unreleased + - Fix the ``constants`` constructor argument silently discarding ``Constants`` *subclass* instances: the exact-type check replaced them with fresh defaults, throwing away the caller's configuration. Subclass instances are now used as given; anything that is neither ``None`` nor a ``Constants`` instance now raises ``TypeError`` instead of being silently swapped for defaults (closes #226) - Fix ``IndexError`` in ``initials()``/``initials_list()`` when a ``*_list`` attribute was assigned directly with an element containing unnormalized whitespace (e.g. ``name.middle_list = ['Q R']``), bypassing the parser's whitespace normalization (closes #232) - Fix ``HumanName`` acting as its own iterator with a stored cursor: breaking out of a loop, iterating in nested loops, or calling ``len(name)`` mid-loop corrupted subsequent iteration; ``iter(name)`` now returns a fresh independent iterator each time. ``next(name)`` on the instance itself (undocumented) now raises ``TypeError`` — call ``next(iter(name))`` instead (closes #225) - Fix parsing writing back into the ``Constants`` it reads (usually the shared module-level ``CONSTANTS``): pieces derived while parsing a name — period-joined titles/suffixes like ``"Lt.Gov."`` and conjunction-joined pieces like ``"Mr. and Mrs."`` or ``"von und zu"`` — are now tracked per parse instead of being permanently ``add()``-ed to the config, so parse results no longer depend on which names were parsed earlier in the process and parsing no longer mutates shared state across threads diff --git a/nameparser/parser.py b/nameparser/parser.py index e6ee2e6..2f10f55 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -95,7 +95,7 @@ class HumanName: def __init__( self, full_name: str | bytes = "", - constants: Constants = CONSTANTS, + constants: Constants | None = CONSTANTS, encoding: str = DEFAULT_ENCODING, string_format: str | None = None, initials_format: str | None = None, @@ -110,9 +110,14 @@ def __init__( nickname: str | list[str] | None = None, maiden: str | list[str] | None = None, ) -> None: + if constants is None: + constants = Constants() + elif not isinstance(constants, Constants): + raise TypeError( + "constants must be a Constants instance or None, " + f"got {type(constants).__name__}" + ) self.C = constants - if type(self.C) is not type(CONSTANTS): - self.C = Constants() # Lookup entries derived while parsing this instance (period-joined # titles/suffixes like "Lt.Gov.", conjunction-joined pieces like diff --git a/tests/test_constants.py b/tests/test_constants.py index 1a415d3..d18ffad 100644 --- a/tests/test_constants.py +++ b/tests/test_constants.py @@ -25,6 +25,22 @@ def test_add_title(self) -> None: self.m(hn.first, "Awanui-a-Rangi", hn) self.m(hn.last, "Black", hn) + def test_constants_subclass_instance_is_used(self) -> None: + class CustomConstants(Constants): + pass + + c = CustomConstants() + c.titles.add('chancellor') + hn = HumanName("Chancellor Jane Smith", constants=c) + self.assertIs(hn.C, c) + self.m(hn.title, "Chancellor", hn) + self.m(hn.first, "Jane", hn) + self.m(hn.last, "Smith", hn) + + def test_constants_invalid_type_raises_typeerror(self) -> None: + with pytest.raises(TypeError): + HumanName("John Doe", constants="not a Constants") # type: ignore[arg-type] + def test_remove_title(self) -> None: hn = HumanName("Hon Solo", constants=None) start_len = len(hn.C.titles) From 84b81cc9371f2270a28fa72b59bd31085a90b96a Mon Sep 17 00:00:00 2001 From: Derek Gulbranson Date: Sun, 5 Jul 2026 15:44:09 -0700 Subject: [PATCH 2/2] Review follow-ups: class-not-instance hint, stale doc sweep The likeliest wrong constants argument is the Constants class itself (forgotten parentheses), which the TypeError reported as an unhelpful "got type"; special-case classes with a did-you-mean hint, test-driven with pytest.raises match patterns (also tightened the existing bare TypeError check, which any TypeError in __init__ would have satisfied). Sweep prose left stale by this fix: the config module "Potential Gotcha" docstring had the same only-None phrasing corrected in customize.rst; the AGENTS.md mypy note attributed most test-suite errors to the constants=None pattern, an error class the Constants|None annotation eliminated; and the :param constants: docstring now documents the TypeError and drops its malformed type spec. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 2 +- nameparser/config/__init__.py | 7 ++++--- nameparser/parser.py | 13 +++++++++---- tests/test_constants.py | 6 +++++- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 85a70f7..de498f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -160,7 +160,7 @@ Don't use the bare `python3 -m doctest .rst` CLI (no `optionflags`) to che `Constants` class attributes (e.g. `patronymic_name_order`, `middle_name_as_last`) document behavior with a bare string literal placed right after the assignment — Sphinx's attribute-docstring convention. That string never becomes a real `__doc__`, so `--doctest-modules` (which walks `__doc__` attributes) never sees any `.. doctest::` examples inside it — this let a stale example slip through CI once (`middle_name_as_last`, #133). `tests/test_config_attribute_docstrings.py` (#195) closes that gap: it parses `nameparser/config/__init__.py` with `ast` to recover those literals and runs any doctest examples through `doctest.DocTestParser`/`DocTestRunner` explicitly, so `pytest -q` now exercises them too. When adding or editing a `.. doctest::` example in a `Constants` attribute's bare-string docstring, this is the mechanism that actually runs it — don't assume `--doctest-modules` covers it. -**`uv run mypy nameparser/` intentionally excludes `tests/`** (`pyproject.toml`'s `[tool.mypy] packages = ["nameparser"]`) — if you run mypy against `tests/` too you'll see ~40 pre-existing errors; don't treat them as a regression. Most are tests deliberately passing `constants=None` (a documented runtime pattern the `Constants` type hint doesn't capture) or intentionally wrong-typed inputs verifying the code rejects them. +**`uv run mypy nameparser/` intentionally excludes `tests/`** (`pyproject.toml`'s `[tool.mypy] packages = ["nameparser"]`) — if you run mypy against `tests/` too you'll see ~30 pre-existing errors; don't treat them as a regression. Most are intentionally wrong-typed inputs verifying the code rejects them, or `Constants` attribute assignments the config type hints don't capture. **`initials_separator` is intra-group only** — it controls the joiner between consecutive initials *within* a name group (e.g. two middle names in `middle_list`). Spaces *between* groups come from `initials_format`. To fully concatenate initials you need both `initials_separator=""` and `initials_format="{first}{middle}{last}"`. diff --git a/nameparser/config/__init__.py b/nameparser/config/__init__.py index 54732d4..00ad435 100644 --- a/nameparser/config/__init__.py +++ b/nameparser/config/__init__.py @@ -21,9 +21,10 @@ >>> hn.C.titles.add('dean') # doctest: +SKIP >>> hn.parse_full_name() # need to run this again after config changes -**Potential Gotcha**: If you do not pass ``None`` as the second argument, -``hn.C`` will be a reference to the module config, possibly yielding -unexpected results. See `Customizing the Parser `_. +**Potential Gotcha**: If you do not pass ``None`` (or your own +:py:class:`Constants` instance) as the second argument, ``hn.C`` will be a +reference to the module config, possibly yielding unexpected results. See +`Customizing the Parser `_. """ import re import sys diff --git a/nameparser/parser.py b/nameparser/parser.py index 2f10f55..cbbf040 100644 --- a/nameparser/parser.py +++ b/nameparser/parser.py @@ -48,9 +48,10 @@ class HumanName: * :py:attr:`given_names` :param str full_name: The name string to be parsed. - :param constants constants: - a :py:class:`~nameparser.config.Constants` instance. Pass ``None`` for - `per-instance config `_. + :param constants: + a :py:class:`~nameparser.config.Constants` instance (subclasses are + honored). Pass ``None`` for `per-instance config `_. + Anything else raises ``TypeError``. :param str encoding: string representing the encoding of your input :param str string_format: python string formatting :param str initials_format: python initials string formatting @@ -113,9 +114,13 @@ def __init__( if constants is None: constants = Constants() elif not isinstance(constants, Constants): + # passing the class itself is the likeliest mistake, and + # reporting it as "got type" would only add confusion + hint = (" (a class was passed; did you mean Constants()?)" + if isinstance(constants, type) else "") raise TypeError( "constants must be a Constants instance or None, " - f"got {type(constants).__name__}" + f"got {type(constants).__name__}{hint}" ) self.C = constants diff --git a/tests/test_constants.py b/tests/test_constants.py index d18ffad..1a6110b 100644 --- a/tests/test_constants.py +++ b/tests/test_constants.py @@ -38,9 +38,13 @@ class CustomConstants(Constants): self.m(hn.last, "Smith", hn) def test_constants_invalid_type_raises_typeerror(self) -> None: - with pytest.raises(TypeError): + with pytest.raises(TypeError, match="constants must be"): HumanName("John Doe", constants="not a Constants") # type: ignore[arg-type] + def test_constants_class_instead_of_instance_raises_with_hint(self) -> None: + with pytest.raises(TypeError, match=r"did you mean Constants\(\)"): + HumanName("John Doe", constants=Constants) # type: ignore[arg-type] + def test_remove_title(self) -> None: hn = HumanName("Hon Solo", constants=None) start_len = len(hn.C.titles)