feat: wrap fset/fdel in deprecated property#190
Merged
Merged
Conversation
- deprecation.py: packing() now applied to fset and fdel (with None guards), not just fget — FutureWarning fires on read, write, and delete - test_deprecation.py: three new tests in TestPropertyOrderAgnostic — setter fires warning, deleter fires warning, setter-only property (fget=None) - docs/guide/use-cases.md: new subsection "Deprecating a property with a setter or deleter" with worked example and chain-style warning - docs/llms.txt: Agent Notes bullet for @Property updated to document fset/fdel wrapping and explicit-construction requirement Limitation: @value.setter chain-style does not work — property.setter() rebuilds a fresh property bypassing packing(); explicit property(fget, fset[, fdel]) required. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…r/deleter - deprecation.py: add _DeprecatedProperty(property) subclass that stores a _wrap packing closure and overrides .getter()/.setter()/.deleter() to re-wrap new accessors — enables @value.setter chain-style to fire FutureWarning without explicit property(fget, fset) construction - test_deprecation.py: two new tests — test_chain_style_setter_fires_warning and test_chain_style_deleter_fires_warning — verify both read and write/delete fire FutureWarning in chain-style decorator pattern Both patterns now work identically: explicit: deprecated(...)(property(fget, fset)) chain: @deprecated @Property + @value.setter --- Co-authored-by: Claude Code <noreply@anthropic.com>
- use-cases.md: rewrite "Deprecating a property with a setter or deleter" section to show chain-style (@deprecated @Property + @value.setter) as the primary pattern; retain explicit property(fget, fset) as alternative; drop the now-incorrect warning admonition - llms.txt: update Agent Notes @Property bullet — replace explicit-only guidance with note that both chain-style and explicit construction work via _DeprecatedProperty subclass --- Co-authored-by: Claude Code <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends @deprecated’s order-agnostic @property support so deprecation warnings also fire for property writes (fset) and deletes (fdel), and adds documentation + tests to cover setter/deleter behavior (including chain-style @value.setter / @value.deleter rebinding).
Changes:
- Wrap
propertyaccessorsfget,fset, andfdelinpacking()and introduce_DeprecatedPropertyto preserve wrapping through chain-style.setter()/.deleter(). - Add unit tests asserting
FutureWarningfires on property set/delete, including setter-only properties and chain-style setter/deleter definitions. - Update docs to describe property deprecation behavior across read/write/delete and document chain-style vs explicit construction patterns.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/deprecate/deprecation.py |
Adds _DeprecatedProperty and updates packing() so fset/fdel also emit deprecation warnings and remain wrapped through chain-style rebinding. |
tests/unittests/test_deprecation.py |
Adds tests for warning emission on property setter/deleter and chain-style setter/deleter usage. |
docs/guide/use-cases.md |
Documents deprecating properties with setters/deleters, including examples. |
docs/llms.txt |
Updates agent notes to reflect new @property wrapping behavior (including chain-style support). |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…imeout Setter/deleter section used KeyPoints.confidence → keypoint_confidence which required computer vision domain knowledge. Replaced with Config.timeout → connect_timeout — universally readable rename. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…est type annotations - docs/guide/use-cases.md: second code block (explicit property construction) was indented with 4 spaces inside a fenced block, causing phmdoctest to generate syntactically-invalid Python (IndentationError at module import time → entire tests/docs/test_guide_use_cases.py collection failed). Added <!--phmdoctest-skip--> directive and rewrote block as a complete self-contained class example with import - tests/unittests/test_deprecation.py: five accessor functions used `self: Any` (ruff ANN401); three `deprecated()(property(...))` calls lacked `# type: ignore[arg-type]`; two property setter assignments lacked `# type: ignore[assignment]`. Changed `self: Any` to `self: object` throughout, added targeted type-ignore comments; all five new tests still pass --- Co-authored-by: Claude Code <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Raise TypeError early when packing() sees isinstance(source, _DeprecatedProperty), preventing silent double-wrapping of all three accessors (two FutureWarnings per access) and spurious _warn_stacking_misconfiguration calls. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…operty Raise TypeError at decoration time when args_mapping, args_extra, or a callable target is combined with a property source — fset signature differs from fget, so _build_call_plan and _check_cross_class_method_target would silently misbehave. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…allable], Callable] Remove the extra _sl default parameter; capture _stacklevel+1 in a local variable so _wrap_accessor's signature matches the _wrap attribute type annotation exactly. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…setter/deleter Mirrors built-in property behaviour: when the newly-supplied accessor carries a docstring, use it; fall back to self.__doc__ only when fnew.__doc__ is absent. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Replace RST :: literal block with Example: section; add Args: entries for fget/fset/fdel/doc/_wrap so the class is fully documented for contributors. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Verify stacklevel arithmetic is correct for chain-style setter — the emitted FutureWarning filename must match the test file, not deprecation internals. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…write New test_chain_style_setter_warns_once: first write fires FutureWarning; second write must be silent once the num_warns=1 state is exhausted. --- Co-authored-by: Claude Code <noreply@anthropic.com>
… class name Removes the _DeprecatedProperty class name from the AI-agent contract to avoid clients pinning a private implementation detail. Adds note on type() vs isinstance(). --- Co-authored-by: Claude Code <noreply@anthropic.com>
Add __init__ with self._timeout = 30 and replace self.connect_timeout references with self._timeout so the example runs without AttributeError. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Outer-order @deprecated @Property now wraps fset and fdel with FutureWarning. Note migration path for filterwarnings=error::FutureWarning consumers. --- Co-authored-by: Claude Code <noreply@anthropic.com>
M3: reword _wrap_accessor comment to list only meaningful captured params (template_mgs, stream, deprecated_in, remove_in, num_warns, skip_if); args_mapping/args_extra/target are blocked by TypeError guards above. M7: replace "conventional Python decorator pattern" with accurate label clarifying outer order is required for fset/fdel wrapping, not just conventional. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…only Eliminates silent-degrade footgun where .setter()/.deleter() became no-ops when _wrap=None (accessor returned untouched, silently losing deprecation wrapping). _wrap is always supplied by packing() so the Optional default was never needed. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…branch property(deprecated_fget) wrapped again with @deprecated would double-wrap fget, emitting two FutureWarnings per read. The existing _DeprecatedProperty guard only caught property-objects already wrapped; this adds a per-accessor __deprecated__ check before the recursive packing() calls. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…ecation M4: document covariant return type limitation in _DeprecatedProperty docstring (setter/deleter return _DeprecatedProperty narrowing property; variables typed as plain property lose chain inference at static analysis time). M5: add migration tip to use-cases.md setter/deleter section explaining how to get a silent setter/deleter when only the getter needs to warn. --- Co-authored-by: Claude Code <noreply@anthropic.com>
…ME and llms.txt G15: README line clarifies cached_property is getter-only (no fset/fdel); was misleadingly grouped with @Property's three-accessor wrapping. G16: llms.txt flowchart splits @Property and @cached_property into separate branches; cached_property has no setter/deleter so the all-three-accessors description was incorrect for it. --- Co-authored-by: Claude Code <noreply@anthropic.com>
- H2: inner-order (@Property @deprecated) setter and deleter do NOT warn — InnerOrderDeprecatedPropCls fixture + two regression tests confirm chain-style @value.setter / @value.deleter are plain callables when inner order used - H3: fdel-only property(None, None, fdel) fires FutureWarning on del + audit discovery — DelOnlyDeprecatedPropCls fixture, deleter_only runtime test, and test_finds_deleter_only_property in test_audit.py - M6: all three accessors fire independently + per-accessor counter independence + validate_deprecation_expiry with deprecated property --- Co-authored-by: Claude Code <noreply@anthropic.com>
- M8: two new troubleshooting sections in docs/troubleshooting.md:
1. "Setter or deleter on a deprecated property doesn't warn" — explains inner
vs outer order, shows both patterns and the explicit property(fget,fset,fdel)
alternative
2. "TypeError raised when applying @deprecated to an already-deprecated property"
— explains the double-wrap guard introduced in M2 and how to fix it
- Matching FAQPage JSON-LD pairs added to docs/overrides/main.html for both entries
---
Co-authored-by: Claude Code <noreply@anthropic.com>
Move # type: ignore[arg-type] to the property(...) call line where mypy reports the error; mdformat trailing-whitespace cleanup in troubleshooting.md. --- Co-authored-by: Claude Code <noreply@anthropic.com>
… property - Add guard in the property branch of packing() that raises TypeError when target=TargetMode.ARGS_REMAP or legacy target=True — self-deprecation mode requires a function signature and has no meaningful semantics for property accessors; previously these values silently flowed into accessor wrapping - Add parametrized test covering both TargetMode.ARGS_REMAP and True legacy sentinel --- Co-authored-by: Claude Code <noreply@anthropic.com>
…orate guard - Replace hasattr(acc, '__deprecated__') with _has_deprecation_meta(acc) in the per-accessor pre-decoration check inside the property branch of packing(); hasattr is true for any __deprecated__ value whereas _has_deprecation_meta checks isinstance(acc.__deprecated__, DeprecationConfig), preventing false positives from unrelated attributes with the same name - Use getattr(acc, '__qualname__', repr(acc)) in the TypeError message to satisfy mypy after TypeGuard narrows _acc to _HasDeprecationMeta (which only declares __deprecated__) - Add test verifying an accessor with __deprecated__ set to a non-DeprecationConfig value passes through without raising --- Co-authored-by: Claude Code <noreply@anthropic.com>
- Add "Deprecated property alias on a dataclass" subsection under Properties and cached properties — covers read-only alias (inner order) and read-write alias (outer order + .setter chain), with warning about shadowing existing dataclass fields - Add "Redirecting through a deprecated property" subsection — explains why target=<callable> raises TypeError on property, and shows the manual delegation pattern in the accessor body - Sync docs/llms.txt Decision Flowchart @Property branch and Agent Notes bullet: document target=<callable>/ARGS_REMAP/True rejection, manual redirect pattern, and dataclass alias constraint --- Co-authored-by: Claude Code <noreply@anthropic.com>
- Add instance-level usage (create, set, read, delete) to three property code blocks in use-cases.md - Add <details><summary>Output: …</summary> blocks with verified values for all four print() calls - Add output-block rule to AGENTS.md Community-Scribe responsibilities so it is visible at anchor time --- Co-authored-by: Claude Code <noreply@anthropic.com>
…IBUTING - Strip print() wrapper from <summary> labels in use-cases.md — show expression only (e.g. cfg.timeout, not print(cfg.timeout)) - Clarify <details> summary format in CONTRIBUTING.md: label = expression, not print() wrapper --- Co-authored-by: Claude Code <noreply@anthropic.com>
…examples - Reframe Properties section intro: @deprecated only adds FutureWarning, does not forward reads/writes - Reframe dataclass alias intro: delegation is plain Python in accessor body, not provided by library - Fix read-write alias caption: split warning (from @deprecated) vs arithmetic (from user body) - Drop redundant "Deprecated property alias on a regular class" section (identical pattern to dataclass alias) - Rename "Redirecting through a deprecated property" → accurate framing; drop false redirect claim - Trim getter-only example from 4 variants to 2 (inner order only); outer order covered by setter/deleter section - Move target=<callable> unsupported note to where it is most discoverable --- Co-authored-by: Claude Code <noreply@anthropic.com>
…io docstrings - Move TestDescriptorOrderAgnostic, TestPropertyOrderAgnostic, TestPropertyErrorPaths (624 lines) from test_deprecation.py to tests/unittests/test_property.py - Remove no-longer-needed imports from test_deprecation.py: types, Optional, cached_property, validate_deprecation_expiry, InnerOrderDeprecatedPropCls, DelOnlyDeprecatedPropCls, _DeprecatedProperty - Add prose-paragraph scenario descriptions to TestPropertyErrorPaths test docstrings - Require scenario description prose paragraph in all non-trivial test method docstrings (AGENTS.md + CONTRIBUTING.md) --- Co-authored-by: Claude Code <noreply@anthropic.com>
Borda
added a commit
that referenced
this pull request
Jun 5, 2026
- Added deprecation support for property setters and deleters (`fset`/`fdel`) in addition to getters - Introduced a property subclass that preserves deprecation wrapping across `.setter()` and `.deleter()` chains - Enabled chain-style `@deprecated @property` patterns to warn on read, write, and delete operations - Added decoration-time guards against double-decorating deprecated properties - Added decoration-time guards rejecting unsupported property targets (`ARGS_REMAP`, `True`, callable targets, `args_extra`) - Added guards against wrapping already-deprecated property accessors - Fixed audit scanning to discover setter-only, deleter-only, and setter-deprecated properties - Improved property warning stacklevel attribution and per-accessor warning-budget behavior - Added independent warning handling for getter, setter, and deleter accessors - Added support and audit coverage for setter-only and deleter-only properties - Added regression coverage for inner-order vs outer-order decorator behavior - Improved property error messages and property-name resolution in TypeErrors - Refined property wrapping internals, type signatures, and docstring propagation behavior - Added extensive property runtime, audit, and error-path test coverage - Extracted property tests into a dedicated `test_property.py` suite - Expanded documentation for property deprecation patterns, decorator ordering, dataclass aliases, and migration guidance - Added troubleshooting guidance for property-order pitfalls and double-wrap errors - Clarified `@property` vs `@cached_property` accessor coverage - Added documented patterns for deprecated property aliases and manual delegation - Updated audit, README, changelog, LLM guidance, and use-case examples to reflect full property support - Fixed documentation examples, doctest execution issues, typing/lint problems, and review follow-ups --------- Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Borda
added a commit
that referenced
this pull request
Jun 5, 2026
- Added deprecation support for property setters and deleters (`fset`/`fdel`) in addition to getters - Introduced a property subclass that preserves deprecation wrapping across `.setter()` and `.deleter()` chains - Enabled chain-style `@deprecated @property` patterns to warn on read, write, and delete operations - Added decoration-time guards against double-decorating deprecated properties - Added decoration-time guards rejecting unsupported property targets (`ARGS_REMAP`, `True`, callable targets, `args_extra`) - Added guards against wrapping already-deprecated property accessors - Fixed audit scanning to discover setter-only, deleter-only, and setter-deprecated properties - Improved property warning stacklevel attribution and per-accessor warning-budget behavior - Added independent warning handling for getter, setter, and deleter accessors - Added support and audit coverage for setter-only and deleter-only properties - Added regression coverage for inner-order vs outer-order decorator behavior - Improved property error messages and property-name resolution in TypeErrors - Refined property wrapping internals, type signatures, and docstring propagation behavior - Added extensive property runtime, audit, and error-path test coverage - Extracted property tests into a dedicated `test_property.py` suite - Expanded documentation for property deprecation patterns, decorator ordering, dataclass aliases, and migration guidance - Added troubleshooting guidance for property-order pitfalls and double-wrap errors - Clarified `@property` vs `@cached_property` accessor coverage - Added documented patterns for deprecated property aliases and manual delegation - Updated audit, README, changelog, LLM guidance, and use-case examples to reflect full property support - Fixed documentation examples, doctest execution issues, typing/lint problems, and review follow-ups --------- Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.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.
Details
What does this PR do?
Limitation: @value.setter chain-style does not work — property.setter() rebuilds a fresh property bypassing packing(); explicit property(fget, fset[, fdel]) required.
closes
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃