Skip to content

enum_ QoL improvements and enum.IntEnum #530

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
aldanor opened this issue Nov 25, 2016 · 8 comments
Closed

enum_ QoL improvements and enum.IntEnum #530

aldanor opened this issue Nov 25, 2016 · 8 comments

Comments

@aldanor
Copy link
Member

aldanor commented Nov 25, 2016

I've been using Python 3 enum.IntEnum lately instead of pybind11 enum_<> enums (although they're less straightforward to bind), since the former are (a) faster and (b) more ergonomic/Pythonic.

I wonder if we could either improve enum_ a little (a lot?), and/or provide a shortcut to bind standard library enums on Python 3?

I'll try to prove my point using this mini-example:

#include <pybind11/pybind11.h>

#include <stdexcept>

enum class Side1 : char { Right = 1, Left = -1 };
enum class Side2 : char { Right = 1, Left = -1 };

namespace pybind11 { namespace detail {
template<> struct type_caster<::Side1> {
    static handle cast(const ::Side1& src, return_value_policy, handle) {
        // this is the only flaky-ish part here; a better way to share `test_enums.Side1`?
        static object cls = module::import("test_enums").attr("Side1");
        if (!cls) throw std::runtime_error("unable to import test_enums.Side1");

        static object v_right = cls.attr("Right"), v_left = cls.attr("Left");

        switch (src) {
        case ::Side1::Right: return v_right.inc_ref();
        case ::Side1::Left: return v_left.inc_ref();
        default: return {};
        }
    }

    static PYBIND11_DESCR name() { return _("Side1"); }
};
} }

PYBIND11_PLUGIN(test_enums) {
    namespace py = pybind11;
    using namespace pybind11::literals;
    py::module m("test_enums");
    m.attr("Side1") = py::module::import("enum").attr("IntEnum")
        ("Side1", py::dict("Right"_a=1, "Left"_a=-1));
    py::enum_<Side2>(m, "Side2")
        .value("Right", Side2::Right).value("Left", Side2::Left);
    m.def("f1", []() { return Side1::Left; });
    m.def("f2", []() { return Side2::Left; });
    return m.ptr();
}
  1. Casting is 3x-4x faster:
>>> from test_enums import *
>>> %timeit f1()
10000000 loops, best of 3: 84.7 ns per loop
>>> %timeit f2()
1000000 loops, best of 3: 305 ns per loop
  1. Comparison is 4x-5x faster:
>>> e1, e2 = f1(), f2()
>>> %timeit e1 == e1
10000000 loops, best of 3: 40 ns per loop
>>> %timeit e2 == e2
1000000 loops, best of 3: 212 ns per loop
  1. singleton values versus new instances:
>>> f1() is f1()
True
>>> f2() is f2()
False
  1. enum.IntEnum type:
>>> list(Side1)
[<Side1.Left: -1>, <Side1.Right: 1>]  # ease of enumeration over values
>>> dict(Side1.__members__)
{'Left': <Side1.Left: -1>, 'Right': <Side1.Right: 1>}
  1. enum_ values:
>>> Side2.Left.Left.Left.Left.Left.Left.Left  # stop showing up in my tab completion!
Side2.Left
>>> int(Side2.Left)
TypeError: __int__ returned non-int (type str)  # because `char` caster, I guess...
>>> isinstance(Side2.Left, int)
False
  1. enum.IntEnum values:
>>> isinstance(Side1.left, int)  # no need to reinvent the wheel, IntEnum is a subclass of int
True
>>> Side1.Left.name
'Left'
>>> Side1.Left.value
-1
@aldanor
Copy link
Member Author

aldanor commented Nov 25, 2016

And finally (of course):

  1. Binary size

@wjakob
Copy link
Member

wjakob commented Nov 25, 2016

Hi @aldanor,

this sounds generally reasonable, except for two parts:

  1. In C++ (and thus, also in the Python bindings derived from the C++ code), the types associated with enumerations provide an extra layer of type safety. This type safety would need to be retained in a potential switch to enum.IntEnum.

  2. enum doesn't seem to exist in Python 2. Generally we've tried to keep the Python 2.x and 3.x parts of pybind11 100% in sync, so this is somewhat problematic.

Wenzel

@aldanor
Copy link
Member Author

aldanor commented Nov 25, 2016

enum doesn't seem to exist in Python 2.

It also doesn't exist in the standard library before Python 3.4 -- however, there's enum34 backport which users may want to install or not, it would be entirely their responsibility and not ours. So, the existing enum_<> has to exist in some form anyway in order to provide rudimentary enum support for those poor souls stuck on Python 2.

That being said, if you're compiling a pybind11 extension for Python 3.4-3.5-3.6, you already know that enum module is going to be there 100%.

In C++ (and thus, also in the Python bindings derived from the C++ code), the types associated with enumerations provide an extra layer of type safety. This type safety would need to be retained in a potential switch to enum.IntEnum.

Could you elaborate on that?

switch to enum.IntEnum

I don't think we can make a switch (because Python 2 / <3.4) unless we ship the backport with pybind, so it would likely be an additional feature if we decide on it (e.g. py::enum34<>(...)).

@wjakob
Copy link
Member

wjakob commented Nov 25, 2016

Aha, I assumed that you proposed to replace enum_ with a new implementation (at least conditionally in Python >= 3.4) -- never mind then.

@aldanor
Copy link
Member Author

aldanor commented Mar 3, 2017

@wjakob @jagerman Given recent interest in this, I could get on to implementing it.

If I remember correctly, there were two questions I couldn't answer back then: what's the best way to dynamically (no template specialization) register a type caster for a Python type that has not been created via pybind machinery? Functionally, it would be like the example above, with casting/loading working both ways.

The second question is -- once the type is created, where to store it? (in the example above, you see the module::import hack which must go away).

@aldanor aldanor mentioned this issue Apr 5, 2017
8 tasks
@anntzer
Copy link
Contributor

anntzer commented Nov 14, 2017

Just a note in passing: Py3.6 introduced enum.Flag and enum.IntFlag (https://docs.python.org/3/library/enum.html#intflag), which are Python-level enums which support bitwise-operations with a nice repr.

It would be appreciated if pybind11 could optionally represent enums using Flag and IntFlag as well.

@bstaletic
Copy link
Collaborator

Since #781 got closed, we can close this issue as well.

@AWhetter
Copy link
Contributor

AWhetter commented Nov 24, 2020

Although the original PR got closed, I think that it is still worth aiming to resolve this issue. Some discussion would need to happen around exactly how it would get implemented.
The main reason that it looks like the PR got closed was because it was trying to implement this functionality with existing enums, but that raises lots of backwards compatibility questions. So should this new functionality exist on the existing enum_ class or should it be its own type? 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.
Once that question has been answered we can think about questions like should you be able to add methods to them, should you be allowed to select the type of enum to create, and what should the interface for creating these classes look like?

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 a pull request may close this issue.

5 participants