Skip to content

Commit 84b81cc

Browse files
derek73claude
andcommitted
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 <noreply@anthropic.com>
1 parent 4097e7b commit 84b81cc

4 files changed

Lines changed: 19 additions & 9 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

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: 9 additions & 4 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
@@ -113,9 +114,13 @@ def __init__(
113114
if constants is None:
114115
constants = Constants()
115116
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 "")
116121
raise TypeError(
117122
"constants must be a Constants instance or None, "
118-
f"got {type(constants).__name__}"
123+
f"got {type(constants).__name__}{hint}"
119124
)
120125
self.C = constants
121126

tests/test_constants.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,13 @@ class CustomConstants(Constants):
3838
self.m(hn.last, "Smith", hn)
3939

4040
def test_constants_invalid_type_raises_typeerror(self) -> None:
41-
with pytest.raises(TypeError):
41+
with pytest.raises(TypeError, match="constants must be"):
4242
HumanName("John Doe", constants="not a Constants") # type: ignore[arg-type]
4343

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+
4448
def test_remove_title(self) -> None:
4549
hn = HumanName("Hon Solo", constants=None)
4650
start_len = len(hn.C.titles)

0 commit comments

Comments
 (0)