Skip to content

feat(proxy): add attrs_mapping to deprecated_class for fine attribute deprecation#191

Open
Borda wants to merge 49 commits into
mainfrom
feat/cls-atr
Open

feat(proxy): add attrs_mapping to deprecated_class for fine attribute deprecation#191
Borda wants to merge 49 commits into
mainfrom
feat/cls-atr

Conversation

@Borda

@Borda Borda commented Jun 5, 2026

Copy link
Copy Markdown
Owner
Before submitting
  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

  • Add attrs_mapping: Optional[dict[str, Optional[str]]] to deprecated_class() and _DeprecatedProxy
  • __getattr__/__setattr__/__delattr__ warn only for listed attribute names; unlisted attrs pass through silently
  • Redirect reads/writes/deletes to canonical attribute name when value is non-None; warn-only when value is None
  • Reuse existing _warn(arg_name=...) per-key budget path for per-attribute warning counters
  • Raise ValueError at decoration time for circular mappings (key also appears as a redirect target)
  • Store attrs_mapping in both DeprecationConfig (audit visibility) and _ProxyConfig (runtime)
  • Add TestDeprecatedAttrs with 10 tests covering read/write/delete redirect, notify-only, per-attr budget, enum, message content, circular validation

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 🙃

Borda and others added 2 commits June 5, 2026 14:30
…ibute deprecation

- Add `attrs_mapping: Optional[dict[str, Optional[str]]]` to `deprecated_class()` and `_DeprecatedProxy`
- `__getattr__`/`__setattr__`/`__delattr__` warn only for listed attribute names; unlisted attrs pass through silently
- Redirect reads/writes/deletes to canonical attribute name when value is non-None; warn-only when value is None
- Reuse existing `_warn(arg_name=...)` per-key budget path for per-attribute warning counters
- Raise `ValueError` at decoration time for circular mappings (key also appears as a redirect target)
- Store `attrs_mapping` in both `DeprecationConfig` (audit visibility) and `_ProxyConfig` (runtime)
- Add `TestDeprecatedAttrs` with 10 tests covering read/write/delete redirect, notify-only, per-attr budget, enum, message content, circular validation

---
Co-authored-by: Claude Code <noreply@anthropic.com>
- Add "Selective attribute deprecation" section to docs/guide/use-cases.md with example, output block, and audit-visibility note
- Add attrs_mapping <details> example block to README.md Enums/dataclasses section; add attrs_mapping to deprecated_class param tip
- Add recipe, Decision Table row, flowchart branch, numbered decision step, and Agent Notes entry to docs/llms.txt

---
Co-authored-by: Claude Code <noreply@anthropic.com>
@Borda Borda marked this pull request as ready for review June 5, 2026 20:30
Copilot AI review requested due to automatic review settings June 5, 2026 20:30
@dosubot dosubot Bot added documentation Improvements or additions to documentation enhancement New feature or request tests labels Jun 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends deprecated_class() / _DeprecatedProxy with an attrs_mapping option to selectively deprecate individual attribute names (warn + optional redirect) while allowing all other attribute access to pass through silently. This fits pyDeprecate’s proxy-based class deprecation model by enabling fine-grained attribute-level migrations without changing the class-level deprecation mechanism.

Changes:

  • Add attrs_mapping: Optional[dict[str, Optional[str]]] support end-to-end (runtime proxy config + __deprecated__ metadata) with decoration-time circular mapping validation.
  • Implement selective warning/redirect behavior in _DeprecatedProxy.__getattr__, __setattr__, and __delattr__, plus corresponding warning message formatting in _warn().
  • Add new fixtures + unit tests for read/write/delete redirect, warn-only behavior, per-attribute warning budgets, enum member redirect, and metadata visibility; update docs with examples.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/deprecate/proxy.py Adds attrs_mapping plumbing, validation, selective attr forwarding, and per-attr warning formatting.
src/deprecate/_types.py Extends DeprecationConfig / _ProxyConfig to store attrs_mapping for audit/runtime visibility.
tests/unittests/test_proxy.py Adds a dedicated TestDeprecatedAttrs suite to validate selective attribute deprecation behavior.
tests/collection_targets.py Introduces TargetPalette / TargetPaletteEnum as targets for attribute-mapping proxy tests.
tests/collection_deprecate.py Adds module-level deprecated_class(..., attrs_mapping=...) fixtures for the new tests.
README.md Documents attrs_mapping as a deprecated_class() option and provides a runnable example.
docs/llms.txt Updates LLM-oriented usage notes and decision guide to mention attrs_mapping.
docs/guide/use-cases.md Adds a “Selective attribute deprecation” section with an example (needs correction per review comments).

Comment thread src/deprecate/proxy.py Outdated
Comment thread src/deprecate/proxy.py
Comment thread src/deprecate/proxy.py
Comment thread src/deprecate/proxy.py Outdated
Comment thread docs/guide/use-cases.md Outdated
Borda and others added 13 commits June 5, 2026 14:58
…ntax

- Add @deprecated_class(attrs_mapping=...) decorator-syntax example as primary pattern in use-cases.md
- Add four new subsections: read/write/delete redirect, warn-only (None), per-attribute independent budgets, Enum member alias
- Fix Enum example to use wrapper form wrapping canonical Direction enum (decorator form caused incorrect is-comparison across enum classes)
- Expand README attrs_mapping <details> block with decorator and wrapper forms plus cross-link to docs
- All examples verified against live library; mkdocs build --strict passes

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…mapping

- Append `, attrs_mapping=None` after `template_mgs=None` in both DeprecationWrapperInfo repr lines in docs/guide/audit.md
- Fixes make docs-tests CI failure caused by stale repr snapshot after attrs_mapping field was added to DeprecationConfig

[resolve #6] /review finding by foundry:doc-scribe (report: .reports/review/2026-06-05T21-23-29Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
… path

- Change URL from .../use-cases/#selective-attribute-deprecation to .../stable/guide/use-cases.html#selective-attribute-deprecation
- Deployed docs use use_directory_urls=false format; anchor is in the guide subdirectory
- Fixes links-check CI failure (lychee 404 on wrong path format)

[resolve #7] /review finding by foundry:doc-scribe (report: .reports/review/2026-06-05T21-23-29Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…apping

- Old check (set(keys) & {non-None values}) incorrectly rejects valid multi-stage rename chains like {"a":"b","b":"c"} — b appears as both key and redirect target but no loop exists at runtime
- New DFS walk follows each key's redirect chain and only raises ValueError when the chain circles back to a previously-visited key
- Valid patterns now accepted: two-stage migration {"legacy":"old","old":"new"}, fan-in {"color":"hue","colour":"hue"}
- Cycles still rejected: {"a":"b","b":"a"}, {"a":"b","b":"c","c":"a"}
- Update deprecated_class docstring to document the corrected semantics

[resolve #8] /review finding by foundry:sw-engineer (report: .reports/review/2026-06-05T21-23-29Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
… behavior

- Add warning note to attrs_mapping docstring in deprecated_class(): when target=SomeClass is also set, attr redirects land on the target's namespace rather than the source class
- Add PaletteOld source class to collection_targets.py for pinning test
- Add DeprecatedAttrsPaletteCallableTarget fixture to collection_deprecate.py
- Add test_attrs_mapping_with_callable_target_resolves_against_target_namespace to pin the cross-namespace read and write behavior

[resolve #9] /review finding by foundry:challenger (report: .reports/review/2026-06-05T21-23-29Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…ing key prefix

- __getattr__/__setattr__/__delattr__ now pass arg_name="__attr__:<name>" to _warn() when calling for attrs_mapping deprecations; the prefix keeps the warned_args budget entry disjoint from any same-named args_mapping key
- _warn() detects the "__attr__:" prefix, strips it for message formatting (source_name shows bare attr name), but retains the full prefixed key for the warned_args counter increment — ensuring the check and increment use the same bucket
- Fixes M1: with args_mapping={"color":"colour"} AND attrs_mapping={"color":"colour"}, accessing W.color no longer emits the arguments template and no longer shares the warning budget with the constructor-kwarg path

[resolve #1] @Copilot (gh): proxy.py:390 Use distinct internal key in __getattr__ to avoid args_mapping collision
[resolve #2] @Copilot (gh): proxy.py:420 Use distinct internal key in __setattr__ to avoid args_mapping collision
[resolve #3] @Copilot (gh): proxy.py:439 Use distinct internal key in __delattr__ to avoid args_mapping collision
[resolve #4] @Copilot (gh): proxy.py:274 Strip internal key prefix in _warn() before attrs_mapping lookup

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…not .timeout

- Add `size: int = 42` to Config class so it actually has the warn-only attribute
- Change print(DeprecatedConfig.timeout) to print(DeprecatedConfig.size) so the example demonstrates the warn-only behavior it claims to show
- Update Output block: 30 → 42

[resolve #5] @Copilot (gh): use-cases.md:728 Fix use-cases.md example — access .size not .timeout for warn-only demo

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…class at decoration time

- After cycle detection, check that every non-None value in attrs_mapping exists on the class that _get_active() would return: the callable target when one is set, otherwise obj
- Raises ValueError at decoration time with a list of missing attr names rather than silently burning the warning budget on first access and then raising AttributeError
- None-mapped keys (warn-only) are exempt from the check

[resolve #10] /review finding by foundry:sw-engineer (report: .reports/review/2026-06-05T21-23-29Z/review-report.md)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Two new tests in TestDeprecatedAttrs:
- test_delete_redirect_warns_and_deletes_canonical: del proxy.color with {"color":"colour"} warns and deletes canonical attr
- test_delete_notify_only_warns_and_deletes_same_name: del proxy.size with {"size":None} warns and deletes same-name attr

Both tests restore the deleted class attribute in a finally block to avoid test pollution.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
The DeprecationConfig.attrs_mapping docstring stated "Audit tools surface this
for documentation and migration tracking" but audit.py has no per-attribute
fields. Replace with accurate statement that it is stored for future audit
tooling but not yet surfaced.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Two new code blocks in the attrs_mapping <details> section lacked
<details><summary>Output: ...></summary> assertion blocks required by
project convention. phmdoctest generated them as no-assertion tests.
Added correct output blocks and regenerated test_readme.py.

---
Co-authored-by: Claude Code <noreply@anthropic.com>
- Wrap long E501 docstring line in attrs_mapping param docs
- Apply ruff-format reformatting (list comprehension layout, slice spacing)
- Fix import sort order in collection_deprecate.py

---
Co-authored-by: Claude Code <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/deprecate/proxy.py:431

  • __getattr__'s read-only mutator guarding (append, update, etc.) is bypassed whenever attrs_mapping is set because the method returns early for both deprecated and non-deprecated attribute lookups. This breaks the class docstring contract for _DeprecatedProxy (“In read-only mode, common mutating methods … are wrapped …”) and makes read_only=True ineffective for method accesses in the attrs_mapping mode.
        attrs_mapping = self._cfg.attrs_mapping
        if attrs_mapping is not None:
            if name in attrs_mapping:
                self._warn(arg_name=f"__attr__:{name}")
                redirect = attrs_mapping[name]
                active = self._get_active()
                return getattr(active, redirect if redirect is not None else name)
            # Not a deprecated attr — silent passthrough, no warning.
            return getattr(self._get_active(), name)
        self._warn()
        attr = getattr(self._get_active(), name)
        # In read-only mode, guard common mutating methods accessed via attribute lookup.
        if self._cfg.read_only and callable(attr) and self._is_potential_mutator(name):

            def _guarded_mutator(*args: Any, **kwargs: Any) -> None:  # noqa: ANN401
                self._check_read_only(f"Calling mutating method '{name}'")

            return _guarded_mutator
        return attr

Comment thread src/deprecate/proxy.py Outdated
Comment thread tests/unittests/test_proxy.py Outdated
Comment thread src/deprecate/proxy.py Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • src/deprecate/proxy.py — fixed legacy bool target validation and added read-only mutator guard in attrs_mapping path
  • tests/ — added regression tests for legacy bool target and read-only attrs_mapping

Reviewed by step-3.7-flash-20260528 · 650,677 tokens

Borda and others added 16 commits June 6, 2026 11:55
…explicit target

When both mappings are passed without an explicit `target`, auto-resolve
sets `target=TargetMode.ARGS_REMAP` (args_mapping takes precedence) and
`DeprecationConfig.target` no longer reflects that `attrs_mapping` is
also active. Audit tooling cannot detect selective attribute deprecation
from `target` alone. Detect this in `_validate_proxy` using the
previously-unused `args_mapping` parameter and emit a `UserWarning`
directing callers to pass an explicit target.

- _types.py: add ARGS_REMAP + attrs_mapping check to _validate_proxy;
  add doctest for the new case
- test_proxy.py: add test_both_mappings_without_explicit_target_emits_userwarning

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Two code examples imported `warnings` but never called any `warnings.*`
function — the inline `# warns: FutureWarning` annotations are prose,
not code. Remove the dead imports to avoid misleading readers into
thinking the blocks manipulate warning filter state.

- use-cases.md: remove `import warnings` from "Reads, writes, and deletes
  all redirect" block (~line 776) and "Callable target with attribute
  redirection" block (~line 961)

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Runtime verification via warning.filename test revealed stacklevel=5
overshoots to the pytest runner frame. deprecated_class() is already off
the stack when decorator(cls) runs, so the actual chain is only 4 frames
deep: warn → _validate_proxy → __init__ → decorator(cls) → caller.

Also adds test_per_attribute_warning_budget_num_warns_two verifying the
off-by-one boundary at num_warns=2: 3 accesses emit exactly 2 warnings.

- proxy.py: stacklevel=5 → 4 in _validate_proxy call; update comment
- test_proxy.py: add test_validate_proxy_userwarning_points_to_decoration_call_site
- test_proxy.py: add test_per_attribute_warning_budget_num_warns_two

---
Co-authored-by: Claude Code <noreply@anthropic.com>
…lacks it

- Validation: only raise when None-value key absent from BOTH target and source
  (previously raised if absent from target alone, breaking "being-removed" pattern)
- __getattr__: for None-redirect, fall back to cfg.obj (source) when target raises
  AttributeError — enables accessing deprecated attributes not yet on new target
- README.md: add missing size attribute to Config so attrs_mapping example is valid
- Regenerate tests/integration/test_readme.py after README fix

---
Co-authored-by: Claude Code <noreply@anthropic.com>
Per-attribute budget example used `_ = proxy.attr` which only proved no
exception — did not validate redirect returns correct value. Replaced with
`print()` and added `<details>` output block confirming `color`→`colour`
returns `"red"` and `txt`→`text` returns `"hello"`.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Inline `any(...)` guard in `validate_deprecation_wrapper` extracted to
`has_chained_attrs` so the `if` reads as a single line. Switched from
`bool(attrs_mapping)` to `attrs_mapping is not None` to satisfy mypy
type-narrowing on `dict[str, str | None] | None`.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
audit.py: extract `_detect_chain_type` and `_validate_args_mapping` from
`validate_deprecation_wrapper`, reducing it from ~80 to ~30 lines.

proxy.py: extract `_build_attr_warning_msg` from `_warn`; inline comments
become a docstring explaining the `__attr__:` prefix invariant.

tests: replace three `try/finally` save-restore patterns with
`monkeypatch.setattr` — fixture handles teardown even when attribute is
deleted mid-test.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Switch `("proxy", "form")` tuple to single `"proxy"` axis with
`pytest.param(..., id=)` — IDs co-located with values. Drop `form: str`
param and the `assert form in {"decorator", "wrapper"}` tautology that
tested the parametrize list rather than behaviour.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
collection_deprecate.py: extract `_class_deprecation_notify_only_callable_target`
so H5 decorator and wrapper forms share one `deprecated_class(...)` instance
instead of duplicating kwargs.

CONTRIBUTING.md: expand the one-liner note on class pairs into a full
example (with `NewWidget`, `_OriginalWidget`, decorated and wrapped forms)
and add an explicit rule — sharing the instance is mandatory, not optional.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Six private functions across five modules lacked docstrings: `_reasons`,
`_detect_chain_type`, `_validate_args_mapping`, `_wrap_accessor`,
`_update_obj`, `_guarded_mutator`. Added minimal one-line docstrings to
each.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
…urns/Raises, and no-test rule

Private functions now require at minimum a one-line summary. Public
functions require full Args/Returns/Raises sections (omit only when
genuinely inapplicable). Functions with no dedicated test must have at
least one runnable Examples: doctest as the minimum proof of behaviour.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Mirror the rule already in AGENTS.md: never use
`catch_warnings(record=True) + simplefilter("always")` in any .md code
block; use `# warns: FutureWarning` / `# silent` inline annotations instead.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
- Extract `_validate_attrs_mapping`, `_validate_attrs_redirect_targets`, and `_validate_attrs_warn_only_keys` as staticmethods on `_DeprecatedProxy`; `__init__` validation block replaced with a single call
- Reflow all in-code comments in `proxy.py` to 120-char line length
- Rephrase `catch_warnings` rule in `AGENTS.md` to preventive form ("use X instead" vs "replace with X")

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Rename TargetColorEnum → ColorEnum, TargetPalette → Palette,
TargetPaletteEnum → PaletteEnum, TargetWithInjected → WithInjected
across collection_targets.py, collection_deprecate.py, and all test
files — the filename already signals these are target classes.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Merge prematurely-wrapped comment pairs in deprecation.py,
griffe_ext.py, test_audit.py, and test_functions.py. Bullet lists,
section headers, and expected-output blocks intentionally left split.

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Comment thread src/deprecate/proxy.py
Comment thread src/deprecate/proxy.py Outdated
Borda and others added 3 commits June 9, 2026 19:00
[resolve #13] Review by @copilot-pull-request-reviewer[bot] (PR #191):
"attrs_mapping validation runs before legacy bool target values are normalized..."
Challenge: evidence=VALID suggestion=VALID resolution=as-suggested

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
[resolve #14] Review by @copilot-pull-request-reviewer[bot] (PR #191):
"When attrs_mapping is set, __getattr__ returns early and never applies..."
Challenge: evidence=VALID suggestion=VALID resolution=as-suggested

---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
---
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
Comment thread tests/unittests/test_proxy.py Fixed
…lized local variable'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.

Comment thread src/deprecate/proxy.py Outdated
Comment thread src/deprecate/proxy.py
Comment on lines 600 to +608
self._check_read_only(f"Setting attribute '{name}'")
attrs_mapping = self._cfg.attrs_mapping
if attrs_mapping is not None and name in attrs_mapping:
self._warn(arg_name=f"__attr__:{name}")
redirect = attrs_mapping[name]
active = self._get_active()
attr_name = redirect if redirect is not None else name
setattr(active, attr_name, value)
return
Comment thread src/deprecate/proxy.py
Comment on lines 621 to +629
self._check_read_only(f"Deleting attribute '{name}'")
attrs_mapping = self._cfg.attrs_mapping
if attrs_mapping is not None and name in attrs_mapping:
self._warn(arg_name=f"__attr__:{name}")
redirect = attrs_mapping[name]
active = self._get_active()
attr_name = redirect if redirect is not None else name
delattr(active, attr_name)
return
Comment thread src/deprecate/_types.py
Comment on lines +307 to +313
if mode is cls.ARGS_REMAP and args_mapping and attrs_mapping:
messages.append(
f"`deprecated_class` on `{source_name}` provides both `args_mapping` and `attrs_mapping` "
"without an explicit `target`. Auto-resolve set `target=TargetMode.ARGS_REMAP`; "
"`DeprecationConfig.target` no longer reflects that `attrs_mapping` is also active. "
"Pass an explicit `target=<class>` or `target=TargetMode.ATTRS_REMAP` to suppress this warning. "
"This will be `TypeError` in `v1.0`."
Comment thread README.md
Comment on lines +944 to +946
print(Palette.color) # warns → returns "red" (redirected to colour)
print(Palette.colour) # silent passthrough → "red"
print(Palette.size) # silent passthrough → 10
Comment thread README.md
Comment on lines +979 to +981
print(DeprecatedConfig.color) # warns → returns Config.colour ("red")
print(DeprecatedConfig.colour) # silent passthrough ("red")
print(DeprecatedConfig.timeout) # silent passthrough (30)
Borda and others added 2 commits June 9, 2026 19:45
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread src/deprecate/proxy.py
return obj

@classmethod
def _has_static_attribute(cls, obj: Any, name: str) -> bool: # noqa: ANN401
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants