Skip to content

Add 2.0 deprecation warnings for bytes input and SetManager legacy surface#253

Merged
derek73 merged 2 commits into
masterfrom
feature/2.0-deprecation-warnings
Jul 6, 2026
Merged

Add 2.0 deprecation warnings for bytes input and SetManager legacy surface#253
derek73 merged 2 commits into
masterfrom
feature/2.0-deprecation-warnings

Conversation

@derek73

@derek73 derek73 commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

Pre-release sweep so every decided 2.0 removal warns in 1.3.0, the bridge release (holding #252 until this lands):

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 to add_with_encoding(), which had a hardcoded stacklevel=2; the warning pointed at nameparser's own loop instead of the caller.
  • HumanName(bytes) → the constructor assigns through the full_name setter, which had the same hardcoded stacklevel=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 correct stacklevel. Verified empirically with warnings.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:

  • Added the missing .. deprecated:: note to add()'s docstring (only add_with_encoding() had one, but add() is the primary entry point and triggers the same warning).
  • Tightened a weak pytest.deprecated_call(match="set") assertion to match="raw underlying set" (the old pattern matched incidental occurrences of "set" in the message).
  • Added a test for 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.
  • Fixed present-tense wording ("is removed in 2.0") in a couple of test comments to match the future-tense wording used everywhere else ("will be removed in 2.0").

Test plan

  • TDD: all new tests watched failing first (16 failures) before implementation
  • New tests: each warning fires (pytest.deprecated_call), str paths stay silent under simplefilter("error"), discard() ignores missing/normalizes/chains/invalidates the cached union, present-member remove() doesn't warn, mixed present+missing members in one call
  • Three pre-existing bytes tests wrapped in deprecated_call(); suite verified noise-free (no warnings summary), including under -W error::DeprecationWarning
  • Full suite 1446 passed / 4 skipped / 22 xfailed, mypy clean, ruff clean, Sphinx html + doctest builds clean
  • Stacklevel attribution verified empirically for both direct-call and constructor/wrapper paths on the bytes warnings
  • docs/customize.rst updated to mention discard(); release log entries added under Breaking Changes & Deprecations

🤖 Generated with Claude Code

…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>
@derek73 derek73 mentioned this pull request Jul 6, 2026
@codecov-commenter

codecov-commenter commented Jul 6, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.68%. Comparing base (74bae19) to head (0044073).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@derek73 derek73 self-assigned this Jul 6, 2026
@derek73 derek73 added this to the v1.3.0 milestone Jul 6, 2026
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>
@derek73 derek73 merged commit 9b3dadf into master Jul 6, 2026
7 checks passed
@derek73 derek73 deleted the feature/2.0-deprecation-warnings branch July 6, 2026 06:30
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants