Skip to content

Commit 192d35e

Browse files
authored
Merge pull request #237 from derek73/fix/issue-226-constants-subclass
Fix constants argument silently discarding Constants subclasses
2 parents d7ad5b6 + 84b81cc commit 192d35e

6 files changed

Lines changed: 46 additions & 13 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ Don't use the bare `python3 -m doctest <file>.rst` CLI (no `optionflags`) to che
160160

161161
`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.
162162

163-
**`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.
163+
**`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.
164164

165165
**`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}"`.
166166

docs/customize.rst

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,10 +456,11 @@ e.g. parsing names on multiple threads.
456456

457457

458458
If you'd prefer new instances to have their own config values, one shortcut is to pass
459-
``None`` as the second argument (or ``constant`` keyword argument) when
459+
``None`` as the second argument (or ``constants`` keyword argument) when
460460
instantiating ``HumanName``. Each instance always has a ``C`` attribute, but if
461-
you didn't pass something falsey to the ``constants`` argument then it's a
462-
reference to the module-level config values with the behavior described above.
461+
you didn't pass ``None`` (or your own :py:class:`~nameparser.config.Constants`
462+
instance) to the ``constants`` argument then it's a reference to the
463+
module-level config values with the behavior described above.
463464

464465
.. doctest:: module config
465466
:options: +ELLIPSIS, +NORMALIZE_WHITESPACE

docs/release_log.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
Release Log
22
===========
33
* 1.3.0 - Unreleased
4+
- 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)
45
- 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)
56
- 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)
67
- 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

nameparser/config/__init__.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
>>> hn.C.titles.add('dean') # doctest: +SKIP
2222
>>> hn.parse_full_name() # need to run this again after config changes
2323
24-
**Potential Gotcha**: If you do not pass ``None`` as the second argument,
25-
``hn.C`` will be a reference to the module config, possibly yielding
26-
unexpected results. See `Customizing the Parser <customize.html>`_.
24+
**Potential Gotcha**: If you do not pass ``None`` (or your own
25+
:py:class:`Constants` instance) as the second argument, ``hn.C`` will be a
26+
reference to the module config, possibly yielding unexpected results. See
27+
`Customizing the Parser <customize.html>`_.
2728
"""
2829
import re
2930
import sys

nameparser/parser.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ class HumanName:
4848
* :py:attr:`given_names`
4949
5050
:param str full_name: The name string to be parsed.
51-
:param constants constants:
52-
a :py:class:`~nameparser.config.Constants` instance. Pass ``None`` for
53-
`per-instance config <customize.html>`_.
51+
:param constants:
52+
a :py:class:`~nameparser.config.Constants` instance (subclasses are
53+
honored). Pass ``None`` for `per-instance config <customize.html>`_.
54+
Anything else raises ``TypeError``.
5455
:param str encoding: string representing the encoding of your input
5556
:param str string_format: python string formatting
5657
:param str initials_format: python initials string formatting
@@ -95,7 +96,7 @@ class HumanName:
9596
def __init__(
9697
self,
9798
full_name: str | bytes = "",
98-
constants: Constants = CONSTANTS,
99+
constants: Constants | None = CONSTANTS,
99100
encoding: str = DEFAULT_ENCODING,
100101
string_format: str | None = None,
101102
initials_format: str | None = None,
@@ -110,9 +111,18 @@ def __init__(
110111
nickname: str | list[str] | None = None,
111112
maiden: str | list[str] | None = None,
112113
) -> None:
114+
if constants is None:
115+
constants = Constants()
116+
elif not isinstance(constants, Constants):
117+
# passing the class itself is the likeliest mistake, and
118+
# reporting it as "got type" would only add confusion
119+
hint = (" (a class was passed; did you mean Constants()?)"
120+
if isinstance(constants, type) else "")
121+
raise TypeError(
122+
"constants must be a Constants instance or None, "
123+
f"got {type(constants).__name__}{hint}"
124+
)
113125
self.C = constants
114-
if type(self.C) is not type(CONSTANTS):
115-
self.C = Constants()
116126

117127
# Lookup entries derived while parsing this instance (period-joined
118128
# titles/suffixes like "Lt.Gov.", conjunction-joined pieces like

tests/test_constants.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,26 @@ def test_add_title(self) -> None:
2525
self.m(hn.first, "Awanui-a-Rangi", hn)
2626
self.m(hn.last, "Black", hn)
2727

28+
def test_constants_subclass_instance_is_used(self) -> None:
29+
class CustomConstants(Constants):
30+
pass
31+
32+
c = CustomConstants()
33+
c.titles.add('chancellor')
34+
hn = HumanName("Chancellor Jane Smith", constants=c)
35+
self.assertIs(hn.C, c)
36+
self.m(hn.title, "Chancellor", hn)
37+
self.m(hn.first, "Jane", hn)
38+
self.m(hn.last, "Smith", hn)
39+
40+
def test_constants_invalid_type_raises_typeerror(self) -> None:
41+
with pytest.raises(TypeError, match="constants must be"):
42+
HumanName("John Doe", constants="not a Constants") # type: ignore[arg-type]
43+
44+
def test_constants_class_instead_of_instance_raises_with_hint(self) -> None:
45+
with pytest.raises(TypeError, match=r"did you mean Constants\(\)"):
46+
HumanName("John Doe", constants=Constants) # type: ignore[arg-type]
47+
2848
def test_remove_title(self) -> None:
2949
hn = HumanName("Hon Solo", constants=None)
3050
start_len = len(hn.C.titles)

0 commit comments

Comments
 (0)