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

[READY] Implement an enum_ property "name" #1345

Merged
merged 1 commit into from
Apr 7, 2018

Conversation

bstaletic
Copy link
Collaborator

The property returns the enum_ value as a string.
For example:

>>> import module
>>> module.enum.VALUE
enum.VALUE
>>> str(module.enum.VALUE)
'enum.VALUE'
>>> module.enum.VALUE.name
'VALUE'

This is actually the equivalent of Boost.Python "name" property.

@jagerman Pointed out #1122 as it is somewhat related. The reason I didn't propose this change there is because I believe that smaller changes are easier to both reason about and review.

def_property_readonly("name", [m_entries_ptr](Type value) -> pybind11::str {
for (const auto &kv : reinterpret_borrow<dict>(m_entries_ptr)) {
if (pybind11::cast<Type>(kv.second[int_(0)]) == value)
return pybind11::str("{}").format(kv.first);
Copy link
Member

Choose a reason for hiding this comment

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

Going through format here is unnecessary: this can just be: return pybind11::str(kv.first);

@bstaletic
Copy link
Collaborator Author

I have added the documentation and addressed the review comment.

docs/classes.rst Outdated

.. note::

It is also possible to use ``py::str(enum)``, however these accomplish different
Copy link
Member

Choose a reason for hiding this comment

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

Typo: that should just be str(enum) (i.e. no py::), since this is a Python reference.

@jagerman
Copy link
Member

jagerman commented Apr 6, 2018

One minor doc fix, but then it looks good to me.

One other comment to add in support of this PR: Python 3 (or older Pythons with enum34) support the .name property as well, so this addition makes sense to me. (It also fits with #781 assuming the current py::enum_ remains the fallback when enum34 isn't available).

The property returns the enum_ value as a string.
For example:

>>> import module
>>> module.enum.VALUE
enum.VALUE
>>> str(module.enum.VALUE)
'enum.VALUE'
>>> module.enum.VALUE.name
'VALUE'

This is actually the equivalent of Boost.Python "name" property.
@bstaletic
Copy link
Collaborator Author

The py:: has been removed.
This shows that I shouldn't be making PR's after having not slept the previous night.

@jagerman jagerman merged commit 289e5d9 into pybind:master Apr 7, 2018
@bstaletic bstaletic deleted the enum_name branch April 8, 2018 06:03
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.

2 participants