Skip to content

docs: Add a note about calling Python C APIs on py::native_enum #5640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

XuehaiPan
Copy link
Contributor

@XuehaiPan XuehaiPan commented May 5, 2025

Description

Add a note about calling Python C APIs on py::native_enum in the documentation.
Developers should never call PyType_* Python C APIs on the enum types created by py:native_enum, which can cause unexpected issues.

See also:

Suggested changelog entry:

Add a note about calling Python C APIs on `py::native_enum` in the documentation.

@henryiii henryiii requested a review from rwgk May 7, 2025 01:52

The enum types created by ``py::native_enum`` are native Python types, while
the enum types created by the older ``py::enum_`` are C++ pybind11 types.
Developers **SHOULD NOT** call ``PyType_*`` Python C APIs on the enum types
Copy link
Collaborator

Choose a reason for hiding this comment

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

First impression: I'm very confused.

Could you please explain more? E.g. provide an example of what exactly one might want to do with PyType_*?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

Please do not merge the current version. See my comments.

At the moment I'm thinking: What the optree codes does, no one should ever do, for any type they don't own.

@XuehaiPan
Copy link
Contributor Author

I added a comment in metaopt/optree#214 (comment). The problem is that I used the to->tp_flags and PyType_Ready(tp) incorrectly, which is unrelated to py::native_enum migration.

@XuehaiPan XuehaiPan closed this May 7, 2025
@XuehaiPan XuehaiPan deleted the native-enum-python-capi branch May 7, 2025 17:49
@rwgk
Copy link
Collaborator

rwgk commented May 7, 2025

It was eye-opening to see how difficult it is to decide "can I include pybind11/native_enum.h"?

I added a reminder to fix that to #5589

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.

4 participants