Skip to content

Minor follow-on to PR #1334 (Fix enum value's __int__ returning non-int when underlying type is bool or of char type) #3232

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

Merged
rwgk merged 4 commits into from
Aug 31, 2021
Merged

Minor follow-on to PR #1334 (Fix enum value's __int__ returning non-int when underlying type is bool or of char type) #3232

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d548339
Minor tweaks.
rwgk Aug 29, 2021
12c1024
Restoring tests/pybind11_tests.h version from master, removing just t…
rwgk Aug 30, 2021
904ee36
[ci skip] Merge branch 'master' into test_enum_comments
rwgk Aug 31, 2021
d6278ef
More-to-the-point test for Python 3.
rwgk Aug 31, 2021
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
8 changes: 4 additions & 4 deletions tests/test_enum.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,13 @@ def test_duplicate_enum_name():

def test_char_underlying_enum(): # Issue #1331/PR #1334:
assert type(m.ScopedCharEnum.Positive.__int__()) is int
assert int(m.ScopedChar16Enum.Zero) == 0 # int() call should successfully return
Copy link
Copy Markdown
Contributor

@Vigilans Vigilans Aug 31, 2021

Choose a reason for hiding this comment

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

The intent of this test was that when __int__() returns a non-int, int() call will throw a error:

>>> Player.black.__int__()
'\x01'

>>> int(Player.black)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __int__ returned non-int (type str)

So I wrote this call in the test and expected it to successfully return.

Copy link
Copy Markdown
Collaborator Author

@rwgk rwgk Aug 31, 2021

Choose a reason for hiding this comment

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

Thanks. I think that is clear just from the test code itself.

assert int(m.ScopedChar16Enum.Zero) == 0
assert hash(m.ScopedChar32Enum.Positive) == 1
assert m.ScopedCharEnum.Positive.__getstate__() == 1 # return type is long in py2.x
assert m.ScopedCharEnum.Positive.__getstate__() == 1
Copy link
Copy Markdown
Contributor

@Vigilans Vigilans Aug 31, 2021

Choose a reason for hiding this comment

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

Yes, the intent of this test is to exercise __getstate__() call (along with __hash__() call). The intent of this comment was because __getstate__() and __hash__() returns a long type in python 2.x, so

assert type(m.ScopedCharEnum.Positive.__getstate__()) is int

will fail in python 2.x platforms. So I tested them by comparing with 1.

Copy link
Copy Markdown
Collaborator Author

@rwgk rwgk Aug 31, 2021

Choose a reason for hiding this comment

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

Aha! :-)
I changed this, using if env.PY2, so that we get what you originally had in mind for Python 3.

The if env.PY2 branch will get purged when we clean out Python 2 support in a few months.

Unfortunately our pre-commit checks (black, flake8) get upset about type(...) is long. To not make things overly complicated for Python 2 I just kept your == 1 test for Python 2.

assert m.ScopedWCharEnum(1) == m.ScopedWCharEnum.Positive
with pytest.raises(TypeError):
# Enum should construct with a int, even with char underlying type
m.ScopedWCharEnum("0")
# Even if the underlying type is char, only an int can be used to construct the enum:
Copy link
Copy Markdown
Contributor

@Vigilans Vigilans Aug 31, 2021

Choose a reason for hiding this comment

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

The intent of this test, I remembered, was that when enum's underlying type is char, then pybind11's enum not only returns a string, but also could only be constructed with a string. So I wrote a test here to expect it to throw a TypeError instead of returning successfully.

m.ScopedCharEnum("0")


def test_bool_underlying_enum():
Expand Down