Skip to content
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

Standard library enum support via an optional include #2704

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AWhetter
Copy link
Contributor

@AWhetter AWhetter commented Nov 30, 2020

Description

This change is a proof of concept implementation that allows C++ enums to be exposed as enum.Enum objects on the Python side. I've decided to make this change partly to spark a design discussion about what would make the most sense when trying to expose C++ enums as standard library enums.

  • Should this functionality exist using a new class (as done here) or should we try to make it work with enum_? This implementation uses a new object type stdlib_enum. Personally the backwards compatibility problems seem complex enough that making a new enum type is worthwhile, but there's a tradeoff of increased binary size. I think we would want to phase out the old enum class in favour of this new one.
  • Should you be able to add methods to enums? This one does not, but it's inflexible as a result.
  • Should you be allowed to select the type of enum to create (Enum, IntEnum, IntFlag)? If so, how? An optional argument to the constructor seems like the obvious choice.
  • What should the interface for creating these classes look like? Does using a destructor make sense as implemented here, does a finalize() method make more sense? How should that interact with being able to add methods (if at all)?
  • Does an optional include (as done here) make sense or should this be integrated into pybind properly?

TODO: Properly implement name().

Currently there are no docs or tests because the design might change a lot.

Kudos to @aldanor who's design I started from.

Implemented #530

Suggested changelog entry:

TODO upon design decisions

@diiigle
Copy link

diiigle commented Dec 17, 2020

Just bounced over this coming from #253.

@anntzer
Copy link
Contributor

anntzer commented Dec 20, 2020

Should you be allowed to select the type of enum to create (Enum, IntEnum, IntFlag)? If so, how? An optional argument to the constructor seems like the obvious choice.

Being able to pick the base stdlib class is nice, at least for using Flag/IntFlag which have highly readable reprs. In https://gist.github.com/anntzer/96f27c0b88634dbc61862d08bff46d10 (my own implementation of binding-to-stdlib enums), I simply pass the base class as a string, which in theory would even allow using third-party base enum classes with compatible signatures (e.g. for backporting to Py<3.4).

@EricCousineau-TRI
Copy link
Collaborator

Ooh, saw this per @bstaletic's post in #2739 (issue: #2332). Will compare notes.

@EricCousineau-TRI
Copy link
Collaborator

EricCousineau-TRI commented Dec 23, 2020

I spewed some text here: #2332 (comment)

If y'all have a chance, might you be able to weigh in on my opinions there? (i.e. rolling all the functionality into py::enum_ proper, rather than relying on macros / type_caster specialization)

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