Skip to content

Add Python generator support for deprecated interface fields#252

Closed
Pengkun-ZHU wants to merge 1 commit intoros2:kiltedfrom
Pengkun-ZHU:feature/field-deprecation-annotation
Closed

Add Python generator support for deprecated interface fields#252
Pengkun-ZHU wants to merge 1 commit intoros2:kiltedfrom
Pengkun-ZHU:feature/field-deprecation-annotation

Conversation

@Pengkun-ZHU
Copy link
Copy Markdown

@Pengkun-ZHU Pengkun-ZHU commented Mar 25, 2026

Implements the Python generator side of field-level deprecation support. This PR is based on and should be merged as a companion to ros2/rosidl#945, which establishes the IDL annotation pipeline for C and C++ generators.

When a message field is annotated with @deprecated(text='...') in an .idl file, the generated Python message class will:

  • Decorate the property getter and setter with @typing_extensions.deprecated(text) (PEP 702), enabling static analysis tools (mypy, pyright, IDEs) to flag deprecated field usage at development time
  • Emit a DeprecationWarning at runtime when the field is accessed or set

The C extension _msg_support.c.em wraps internal accesses to deprecated fields with DISABLE_DEPRECATED_PUSH/POP to suppress warnings in generated conversion code.

Related

Implements the Python generator side of field-level deprecation support. This PR is based on and should be merged as a companion to ros2/rosidl#945, which establishes the IDL annotation pipeline for C and C++ generators. When a message field is annotated with @deprecated(text=...) in an .idl file, the generated Python message class will: - Decorate the property getter and setter with @typing_extensions.deprecated(text) (PEP 702), enabling static analysis tools (mypy, pyright, IDEs) to flag deprecated field usage at development time - Emit a DeprecationWarning at runtime when the field is accessed or set The C extension (_msg_support.c.em) wraps internal accesses to deprecated fields with DISABLE_DEPRECATED_PUSH/POP to suppress warnings in generated conversion code. Changes: - resource/_msg.py.em: import and apply @_deprecated decorator on getter/setter when field has @deprecated annotation - resource/_msg_support.c.em: suppress deprecation warnings around internal PyObject_GetAttrString/SetAttrString calls - package.xml: add python3-typing-extensions exec_depend - CMakeLists.txt: add TestDeprecated.idl test message and test_deprecated.py regression test

Signed-off-by: Pengkun-ZHU <q1091803103@gmail.com>
@Pengkun-ZHU
Copy link
Copy Markdown
Author

@InvincibleRMC @jmachowinski, I did a Python support following the implementation in ros2/rosidl#945. Could you help give it a review?

@christophebedard
Copy link
Copy Markdown
Member

christophebedard commented Apr 2, 2026

I'll let @InvincibleRMC take a look, but FYI you should target the rolling branch. Please rebase on rolling and then modify the PR to target the rolling branch.

@InvincibleRMC
Copy link
Copy Markdown
Contributor

@Pengkun-ZHU What happens if you do equality checks? Do those emit deprecation warnings? I don't think they should.

@Pengkun-ZHU
Copy link
Copy Markdown
Author

Pengkun-ZHU commented Apr 3, 2026

@Pengkun-ZHU What happens if you do equality checks? Do those emit deprecation warnings? I don't think they should.

Nice catch, I'll make __eq__ use self._<field> instead of getter to avoid deprecation warning. Similarly I'll do it for __repr__. Any more to add?
On the other hand, I do feel __init__ still needs warns if a deprecated field is passed — that seems like the right behavior to alert users who are explicitly constructing messages with deprecated fields.

@Pengkun-ZHU
Copy link
Copy Markdown
Author

I'll let @InvincibleRMC take a look, but FYI you should target the rolling branch. Please rebase on rolling and then modify the PR to target the rolling branch.

Duly noted with many thanks. I'll rebase and modify.

@Pengkun-ZHU
Copy link
Copy Markdown
Author

Closing in favor of a new PR targeting rolling branch.

@Pengkun-ZHU Pengkun-ZHU closed this Apr 3, 2026
@Pengkun-ZHU
Copy link
Copy Markdown
Author

New PR targeting rolling branch is filed:
#256

@christophebedard
Copy link
Copy Markdown
Member

FYI, in the future: You can rebase your branch and then change the target branch of your PR to rolling; no need to re-open a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants