Add 2.0 deprecation warnings for bytes input and SetManager legacy surface#253
Merged
Conversation
…rface 1.3.0 is the bridge release, so every decided 2.0 removal warns in it: - bytes to HumanName/full_name or SetManager.add()/add_with_encoding() warns with a decode-first hint (#245); the encoding kwarg is deprecated with it - SetManager.__call__ warns: it returns the raw underlying set, and mutating that bypasses normalization and cache invalidation (#243) - SetManager.remove() of a missing member warns (raises KeyError in 2.0, matching set.remove); new discard() is the intentional ignore-missing spelling, wired to the same cache invalidation (#243) The option-A-only #243 removals (operators, the Set ABC) deliberately do not warn — that decision is still open for feedback on the issue. Present-member remove(), str input, and everything else stay silent; suite is warning-noise-free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #253 +/- ##
==========================================
+ Coverage 97.52% 97.68% +0.15%
==========================================
Files 13 13
Lines 888 906 +18
==========================================
+ Hits 866 885 +19
+ Misses 22 21 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
SetManager.add()/add_with_encoding() and HumanName's constructor/full_name setter each had a fixed stacklevel that only pointed at the real caller when the bytes-deprecation warning fired on the direct-call path. Going through the wrapper (add() -> add_with_encoding(), __init__ -> setter) added a stack frame, so the warning was misattributed to nameparser's own internals instead of the caller's code. Extract the shared decode+warn logic into a private helper (_add_normalized / _apply_full_name) that each public entry point calls directly with its own correct stacklevel, verified empirically that both call paths now attribute to the user's line. Also add the missing .. deprecated:: note to add()'s docstring, tighten a weak match= test assertion, fix present-tense wording in test comments, and add a test for remove()/discard() with mixed present+missing members in one call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Jul 6, 2026
derek73
added a commit
that referenced
this pull request
Jul 12, 2026
* Add 2.0 deprecation warnings for the 1.4 second-wave sweep (#263) One themed PR, mirroring how the 1.3.0 sweep shipped (#253), so every decided 2.0 removal in the second wave warns ahead of time: - empty_attribute_default assignment (#255): once None support goes, the only legal value left is the default, so assignment now warns via a new descriptor; reads stay silent. - TupleManager/RegexTupleManager unknown-key attribute access (#256): a misspelled or omitted key currently degrades silently (None/ EMPTY_REGEX); now warns naming the miss and the known keys. Excludes single-underscore introspection probes (IPython's _repr_html_, etc.) alongside the existing dunder guard. - HumanName slice __getitem__ and all of __setitem__ (#258): field access by position has no real use case, and item assignment duplicates plain attribute assignment. - SetManager.add_with_encoding() (#245): the method itself now warns on every call regardless of argument type, naming add() as the replacement -- previously only the bytes-decode path warned. - Constants.__setstate__'s legacy-pickle migration shim (#279): warns once per call (not once per skipped key) when it loads a pre-1.3.0 blob, telling users to re-pickle. Same standards as #253: TDD throughout, suite verified noise-free including under -W error::DeprecationWarning, mypy/ruff clean, Sphinx doctest + html builds clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Address multi-agent review findings on PR #286 - Fix conftest.py's empty_attribute_default fixture teardown: the DeprecationWarning-ignore filter wrapped the restore of all 8 scalar config attrs instead of just the one deprecated assignment, which would have silently swallowed any future deprecation warning added to the other 7 -- a silent-failure hunter finding. Scoped to match the already-narrow setup block. - Add a type check to _EmptyAttributeDefaultAttribute.__set__: a cheap, immediate TypeError for non-str/non-None values instead of deferring the whole invariant to 2.0 (type-design-analyzer finding). - Document why __get__(obj=None) returns '' rather than `self` (unlike its _SetManagerAttribute sibling): it's load-bearing for Constants.__repr__'s default-value comparison, not just mypy inference (type-design-analyzer finding). - Fix a stale "~8 public str-typed properties" comment (actually 12) in two test files (comment-analyzer finding). - Add tests: Constants.copy() preserves a customized empty_attribute_default without warning; add_with_encoding()'s str path emits exactly one warning, not two; the legacy-pickle warning fires once per __setstate__ call even with two stale keys, not once per key (test-analyzer findings). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Trim redundant/tangential examples from usage.rst's quick-start doctest len(name), list(name), and name['first'] examples removed: the string-key access is already demonstrated earlier in the same doctest (name["title"]), and len()/list() aren't central to the quick-start walkthrough this section is meant to be. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- 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
Pre-release sweep so every decided 2.0 removal warns in 1.3.0, the bridge release (holding #252 until this lands):
DeprecationWarningwith a decode-first hint when bytes reachesHumanName/full_nameorSetManager.add()/add_with_encoding(); theencodingkwarg documented as deprecated with it. The other bytes paths already crash or raise, so they need no warning.SetManager.__call__(SetManager inherits an API surface it doesn't want; decide its real interface #243): warns — common to both SetManager inherits an API surface it doesn't want; decide its real interface #243 options, so it doesn't prejudge the A/B decision.remove()of a missing member (SetManager inherits an API surface it doesn't want; decide its real interface #243): warns (will raiseKeyErrorin 2.0, matchingset.remove) — also common to both options. Addsdiscard()as the intentional ignore-missing spelling, with the same_on_changecache invalidation; removing present members is unchanged and silent.SetABC/isinstance) — SetManager inherits an API surface it doesn't want; decide its real interface #243 is still open for feedback, and warning now would pre-empt it.Follow-up: stacklevel fix from review
A multi-agent review caught that the bytes-deprecation warnings misattributed their source line whenever they fired through a wrapper call instead of directly:
SetManager.add(bytes)→ delegates toadd_with_encoding(), which had a hardcodedstacklevel=2; the warning pointed at nameparser's own loop instead of the caller.HumanName(bytes)→ the constructor assigns through thefull_namesetter, which had the same hardcodedstacklevel=2; the warning pointed at nameparser's own__init__instead of the caller.Fixed by extracting the shared decode+warn logic into a private helper (
_add_normalized/_apply_full_name) that each public entry point (add()/add_with_encoding(), and the setter/__init__) calls directly with its own correctstacklevel. Verified empirically withwarnings.catch_warnings(record=True)that both the direct-call and constructor/wrapper paths now attribute to the actual caller's line.Also from the same review pass:
.. deprecated::note toadd()'s docstring (onlyadd_with_encoding()had one, butadd()is the primary entry point and triggers the same warning).pytest.deprecated_call(match="set")assertion tomatch="raw underlying set"(the old pattern matched incidental occurrences of "set" in the message).remove()/discard()called with a mix of present and missing members in a single call, confirming the warning and cache invalidation aren't short-circuited.Test plan
pytest.deprecated_call), str paths stay silent undersimplefilter("error"),discard()ignores missing/normalizes/chains/invalidates the cached union, present-memberremove()doesn't warn, mixed present+missing members in one calldeprecated_call(); suite verified noise-free (no warnings summary), including under-W error::DeprecationWarningdocs/customize.rstupdated to mentiondiscard(); release log entries added under Breaking Changes & Deprecations🤖 Generated with Claude Code