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
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