Add tests/ to mypy scope; fix real type gaps it surfaces#250
Merged
Conversation
Widens a handful of source annotations that were too narrow for already-supported behavior (str|None config scalars, list[str] accepted by is_prefix/is_conjunction/is_suffix and __setitem__, bytes accepted by add_with_encoding), and adds targeted type: ignore comments on the lines that deliberately violate the type contract to assert a TypeError/behavior, matching the existing convention in test_constants.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #250 +/- ##
=======================================
Coverage 97.52% 97.52%
=======================================
Files 13 13
Lines 888 888
=======================================
Hits 866 866
Misses 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per code review: the AGENTS.md guidance added in this branch says to widen an annotation rather than ignore when runtime already supports the wider type, but empty_attribute_default is that exact case and was left un-widened. Document the deliberate scope decision (avoids cascading into ~8 public str-typed properties) at each ignore site instead of leaving it unexplained. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mypycurrently only checksnameparser/(packages = ["nameparser"]), sotests/has never been type-checked. Running mypy againsttests/directly surfaced 31 errors.Constants.string_format/Constants.suffix_delimitercan beNone(already documented/used that way) — annotatedstr | None.is_prefix/is_conjunction/is_suffixalready handle alist[str]argument at runtime (isinstance(piece, list)) but were typedpiece: str— widened tostr | list[str].HumanName.__setitem__delegates to_set_list, which acceptsstr | list[str] | None, but was typedvalue: str— widened to match.SetManager.add_with_encodingalready decodesbytesinput at runtime but was typeds: str— widened tostr | bytes.test_override_capitalization_exceptionsreused a regex-valuedTupleManager(meant forregexes) as a stand-in forcapitalization_exceptions(which isstr -> str) — swapped in a semantically correct value instead of suppressing.snap = {}intest_constants.pyneeded an explicitdict[str, Any]annotation since it holds heterogeneous values across branches.TypeErroror exercise a documentedNone-disables-the-feature toggle (e.g.empty_attribute_default = None,regexes.emoji = False). Added# type: ignore[code]on those lines, matching the existing convention already used elsewhere intest_constants.py.empty_attribute_defaultspecifically was not widened tostr | Nonelike its siblings above, even thoughNoneis documented/supported there too — doing so would cascade into changing ~8 public property return types (title,first,middle,last,suffix,nickname,initials()) fromstrtostr | None, a bigger API decision than this PR's scope. Left as# type: ignore[assignment]at each site, with a comment explaining the reasoning.teststo[tool.mypy] packagesinpyproject.tomlso CI'suv run mypynow covers it.AGENTS.md: the Commands section (uv run mypy nameparser/→uv run mypy) and a Gotcha that this PR's own change made stale (it previously said mypy intentionally excludestests/).Test plan
mypy— 0 errors, 35 source files checkedruff check— all checks passedpytest— 1438 passed, 4 skipped, 22 xfailed