Modernize config constant data structures#233
Merged
Conversation
A set of tuples consumed as dict(...) means a duplicate name resolves nondeterministically per process (set iteration order varies with hash randomization). A dict literal makes a duplicate key a visible overwrite at write time and is the honest structure for key/value data. The regexes parameter annotation on Constants.__init__ collapses to Mapping: both TupleManager arms were dict subclasses already covered by it, and the new plain-dict default requires it (iterating a dict yields keys, not pairs, so it never satisfied the Iterable[tuple] arm). Closes #227 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 #233 +/- ##
=======================================
Coverage 97.29% 97.29%
=======================================
Files 13 13
Lines 813 813
=======================================
Hits 791 791
Misses 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Same rationale as REGEXES in the previous commit, minus the nondeterminism: it's key/value data consumed as dict(...), a duplicate key in the pair form is silently last-wins, and the docs already describe it as a dictionary. Anyone iterating it directly now gets keys instead of pairs; use .items(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Set literals arrived in Python 2.7/3.0; the set([...])-around-a-list form predates them. Purely cosmetic: contents verified identical against master for all eight constants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review findings from PR #233: - TupleManager's docstring referred to "the tuple constants", which no longer exist; note the name is historical instead - The :param regexes: docstring named the wrong wrapper (TupleManager instead of RegexTupleManager, whose EMPTY_REGEX fallback is behaviorally significant) and used a self-referential xref; point it at ~regexes.REGEXES like the sibling capitalization entry - :type regexes: now says (name, compiled pattern) so readers know the values must be re.Pattern objects - AGENTS.md said every config module defines a set and described regexes.py as wrapped in a TupleManager; two modules now define dicts and the wrapping happens in Constants, with RegexTupleManager - The CAPITALIZATION_EXCEPTIONS release-log entry was missing its reference (#233), violating the documented release-log convention - customize.rst promised add()/remove() on every constant; scope that to the set-valued ones and note dicts use normal dict operations Co-Authored-By: Claude Fable 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
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.
Closes #227, expanded to cover all the Python 2-era data structures in
nameparser/config. Three commits, ordered from behavior-relevant to purely cosmetic:1.
REGEXES: set of(name, pattern)tuples → dict (#227)REGEXESwas asetof tuples consumed asdict(...)byRegexTupleManager. If two entries ever shared a name, which one won was nondeterministic per process (set iteration order varies with hash randomization). A dict literal makes a duplicate key a visible overwrite at write time.2.
CAPITALIZATION_EXCEPTIONS: tuple of tuples → dictSame rationale minus the nondeterminism (tuples are ordered, so last-wins was at least deterministic — just silent). The docs already described it as a dictionary; now the code agrees.
3.
set([...])→ set literalsCONJUNCTIONS,FIRST_NAME_TITLES,TITLES,NON_FIRST_NAME_PREFIXES,PREFIXES, and the threeSUFFIX_*constants used the pre-2.7set([...])-around-a-list form. Purely cosmetic; contents verified element-for-element identical against master for all eight.Supporting changes
Constants.__init__annotations forregexesandcapitalization_exceptionscollapse toMapping[...] | Iterable[tuple[...]]— the oldTupleManagerunion arms weredictsubclasses already covered byMapping, and the plain-dict defaults require itREGEXESorCAPITALIZATION_EXCEPTIONSdirectly now gets keys instead of pairs — use.items()Verification
uv run pytest: 1344 passed, 22 xfailed (after each commit)uv run mypy: no issues in 13 source files4. Documentation accuracy pass
A multi-agent review of the diff surfaced prose made stale by the conversions (plus a few adjacent pre-existing imprecisions): the
TupleManagerdocstring's reference to "the tuple constants", the:param regexes:line naming the wrong wrapper class, AGENTS.md's description of the config modules, a missing release-log issue reference, and customize.rst promisingadd()/remove()on the dict-valued constants. All fixed in the final commit; Sphinx builds warning-free.🤖 Generated with Claude Code