Fix constants argument silently discarding Constants subclasses#237
Merged
Conversation
__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 <noreply@anthropic.com>
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>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #237 +/- ##
==========================================
+ Coverage 97.25% 97.26% +0.01%
==========================================
Files 13 13
Lines 802 805 +3
==========================================
+ Hits 780 783 +3
Misses 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Jul 6, 2026
5 tasks
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
2 participants
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
HumanName.__init__(type(self.C) is not type(CONSTANTS)) silently replacedConstantssubclass instances with fresh defaults, discarding the caller's configuration without any error (closes constants argument silently discards Constants subclasses #226)None→ private per-instanceConstants(unchanged); anyConstantsinstance including subclasses → used as given; anything else →TypeErrorinstead of a silent swap to defaultsconstantsannotation now admitsNone(always accepted and documented); fixed stale customize.rst prose ("pass something falsey" → onlyNone;constant→constantskeyword typo)Behavior notes
""/0) were silently replaced with fresh defaults; they now raiseTypeError— called out in the release logTest plan
Constantssubclass instance is kept (assertIs) and honored by parsing; a non-Constantsvalue raisesTypeError🤖 Generated with Claude Code