Skip to content

WIP: Adding string constructor for enum #1122

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.

Sign up for GitHub

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

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

WIP: Adding string constructor for enum #1122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/classes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,21 @@ The ``name`` property returns the name of the enum value as a unicode string.
>>> pet_type.name
'Cat'

You can also access the enumeration using a string using the enum's constructor,
such as ``Pet('Cat')``. This makes it possible to automatically convert a string
to an enumeration in an API if the enumeration is marked implicitly convertible
from a string, with a line such as:

.. code-block:: cpp

py::implicitly_convertible<std::string, Pet::Kind>();

Now, in Python, the following code will also correctly construct a cat:

.. code-block:: pycon

>>> p = Pet('Lucy', 'Cat')

.. note::

When the special tag ``py::arithmetic()`` is specified to the ``enum_``
Expand Down
8 changes: 8 additions & 0 deletions include/pybind11/pybind11.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,14 @@ template <typename Type> class enum_ : public class_<Type> {
return m;
}, return_value_policy::copy);
def(init([](Scalar i) { return static_cast<Type>(i); }));
def(init([name, m_entries_ptr](std::string value) -> Type {
pybind11::dict values = reinterpret_borrow<pybind11::dict>(m_entries_ptr);
Copy link
Member

Choose a reason for hiding this comment

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

This isn't your code, but let's clean it up anyway: instead of auto m_entries_ptr = m_entries.inc_ref().ptr(); being a raw, incremented pointer, we can change it to: auto entries = m_entries;. Then all the lambdas (this, plus the others that do a reinterpret_borrow) can just capture entries and can simplify their reinterpret_borrow<dict>(m_entries_ptr) to just entries.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed, much nicer the new way. Thanks!

pybind11::str key = pybind11::str(value);
if (values.contains(key))
return pybind11::cast<Type>(values[key]);
else
throw value_error("\"" + value + "\" is not a valid value for enum type " + name);
}));
def("__int__", [](Type value) { return (Scalar) value; });
#if PY_MAJOR_VERSION < 3
def("__long__", [](Type value) { return (Scalar) value; });
Expand Down
16 changes: 16 additions & 0 deletions tests/test_enum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ TEST_SUBMODULE(enums, m) {
.value("ETwo", ETwo, "Docstring for ETwo")
.export_values();

// test_conversion_enum
enum class ConversionEnum {
Convert1 = 1,
Convert2
};

py::enum_<ConversionEnum>(m, "ConversionEnum", py::arithmetic())
.value("Convert1", ConversionEnum::Convert1)
.value("Convert2", ConversionEnum::Convert2)
;
py::implicitly_convertible<py::str, ConversionEnum>();

m.def("test_conversion_enum", [](ConversionEnum z) {
return "ConversionEnum::" + std::string(z == ConversionEnum::Convert1 ? "Convert1" : "Convert2");
});

// test_scoped_enum
enum class ScopedEnum {
Two = 2,
Expand Down
21 changes: 21 additions & 0 deletions tests/test_enum.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ def test_unscoped_enum():

assert int(m.UnscopedEnum.ETwo) == 2
assert str(m.UnscopedEnum(2)) == "UnscopedEnum.ETwo"
assert str(m.UnscopedEnum("ETwo")) == "UnscopedEnum.ETwo"

# order
assert m.UnscopedEnum.EOne < m.UnscopedEnum.ETwo
Expand All @@ -70,8 +71,28 @@ def test_unscoped_enum():
assert not (2 < m.UnscopedEnum.EOne)


def test_converstion_enum():
assert m.test_conversion_enum(m.ConversionEnum.Convert1) == "ConversionEnum::Convert1"
assert m.test_conversion_enum(m.ConversionEnum("Convert1")) == "ConversionEnum::Convert1"
assert m.test_conversion_enum("Convert1") == "ConversionEnum::Convert1"


def test_conversion_enum_raises():
with pytest.raises(ValueError) as excinfo:
m.ConversionEnum("Convert0")
assert str(excinfo.value) == "\"Convert0\" is not a valid value for enum type ConversionEnum"


def test_conversion_enum_raises_implicit():
with pytest.raises(ValueError) as excinfo:
m.test_conversion_enum("Convert0")
assert str(excinfo.value) == "\"Convert0\" is not a valid value for enum type ConversionEnum"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test fails, since it gives a generic message and TypeError.



def test_scoped_enum():
assert m.test_scoped_enum(m.ScopedEnum.Three) == "ScopedEnum::Three"
with pytest.raises(TypeError):
m.test_scoped_enum("Three")
Copy link
Member

Choose a reason for hiding this comment

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

Add a check here against the specific exception message, via:

with pytest.raises(TypeError) as excinfo:
    # ... the test call
assert str(excinfo.value) == "the expected exception message"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The problem with that is this gives the wrong error message. I need help figuring out why the error message is being swallowed. I've tried stepping through it in Xcode, but finding where exceptions (at least this one) is being caught is tricky. I think it's inside a constructor/destructor somewhere. I think that directly calling the constructor works, but not if it's part of an implicit conversion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests for errors, one that passes (direct construction was okay) and one that fails (implicit construction).

z = m.ScopedEnum.Two
assert m.test_scoped_enum(z) == "ScopedEnum::Two"

Expand Down