Skip to content

fix: accept __index__ objects for unsigned integers without conversion so enum == numpy int works - #6193

Open
quinncheong wants to merge 5 commits into
pybind:masterfrom
quinncheong:fix/unsigned-index-noconvert
Open

quinncheong wants to merge 5 commits into
pybind:masterfrom
quinncheong:fix/unsigned-index-noconvert

Conversation

@quinncheong

@quinncheong quinncheong commented Oct 5, 2026 •

Copy link
Copy Markdown

Description

Closes #6192. Related: #5895.

Since #5887, py::enum_ comparison against a scalar uses typed overloads: (Type, Scalar) and then a catch-all (Type, const object &) that returns false. For an enum with an unsigned underlying type, the integer caster rejects __index__ objects (numpy integers among them) in the no-convert pass, because PyLong_AsUnsignedLong[Long] does not call __index__. The catch-all then matches in the same pass, so member == np.int32(3) is False while np.int32(3) == member is True. Signed underlying types are unaffected because PyLong_AsLong does call __index__.

The caster already calls PyNumber_Index explicitly on PyPy for the same reason. This change runs that path on CPython too when the target type is unsigned, gated on PYBIND11_INDEX_CHECK so objects without __index__ do not pay for an extra raise. The enum test fails on master for the uint32_t enum and passes with the fix; the int32_t enum is the control. test_int_convert and test_numpy_int_convert now also run against new uint_passthrough functions, so the caster change is covered directly.

One side effect: in convert mode, an unsigned target now calls __index__ before __int__, matching signed targets. Previously __int__ won for unsigned (the IntAndIndex case in the tests).

Testing

Ran test_enum.py and test_builtin_casters.py on CPython 3.13 and PyPy 3.9 (7.3.16), plus prek -a. The PyPy-only PyNumber_Index branch is no longer needed on PyPy 7.3.16+ (the tests pass with it disabled), but it is kept in case older 7.3.x builds still need it.

Suggested changelog entry:

  • Fixed enum equality against numpy integers and other __index__ objects when the enum's underlying type is unsigned (regression in 3.1.0). Unsigned integer arguments now accept __index__ objects without implicit conversion, and with conversion __index__ takes precedence over __int__, matching signed integers.

This PR was prepared with an AI coding agent (Claude Code); I reproduced the bug and ran the tests listed above before posting.


📚 Documentation preview 📚: https://pybind11--6193.org.readthedocs.build/

Since pybind#5887 an enum with an unsigned underlying type compares unequal to
numpy integers, because the no-convert pass rejects __index__ objects for
unsigned targets and the catch-all overload returns false. Closes pybind#6192.
The no-convert early return was previously PyPy-only, so clang-tidy never
saw it; readability-braces-around-statements now flags it.
@quinncheong

Copy link
Copy Markdown
Author

Pushed 2fa7d31 for the Clang-Tidy failure (readability-braces-around-statements on the if (!convert) return false; that used to be PyPy-only and now compiles on CPython too).

The other red jobs on the first run were not test failures: 21 were cancelled with "The job was not acquired by Runner of type hosted even after multiple attempts", and one Windows job lost contact with its runner. The test jobs that did run passed, across ubuntu, macos and windows at C++11 through C++23, including the free-threaded builds.

Prepared with an AI coding agent (Claude Code), checked by me.

@henryiii

henryiii commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Fable 5.1 review below. On the points.

For item 1, the __index__ behavior here is correct. It does trump __int__. And returning something different from the two is very rare and confusing anyway.

Both item 1-2 are about tests, adding a test seems fine.

Item 3 does sound simpler.

🤖 AI text below 🤖

The background code review of fix/unsigned-index-noconvert finished. I checked its claims against the diff. Findings, most relevant first:

Worth acting on

  1. Convert-mode precedence change for unsigned targets is unpinned. On CPython, unsigned parameters in convert mode now call __index__ before falling back to __int__ via PyNumber_Long. An object with __int__ returning 42 and __index__ returning 0 previously gave 42 for uint32_t and now gives 0. Signed already behaved this way, so this is arguably a fix, but the commit message and changelog do not mention it and no test covers it.

  2. No direct unsigned caster test. The fix is in the generic integer type_caster at include/pybind11/cast.h:262, but the only regression test goes through enum __eq__. Adding uint_passthrough / uint_passthrough_noconvert to tests/test_builtin_casters.cpp and running the existing __index__ matrix against it would cover the real change and pin finding 1.

  3. Extra wasted TypeError in convert mode. For objects with no __index__ (for example Decimal), the unsigned path now raises and clears once in PyNumber_Index, again in as_unsigned, then succeeds in PyNumber_Long. Gating the new branch on PYBIND11_INDEX_CHECK(src.ptr()) avoids the first raise.

Style

  1. #if/#else splits a single if head. A constexpr bool flag selected by the preprocessor, then one if, is easier to read and safer for clang-format.

  2. Comment still cites PyPy 7.3.7's 3.8, but v3 requires 3.9+. Either the PyPy-only branch is dead weight, or the comment should cite the still-affected version.

Ignore

  1. The review flagged missing Assisted-by: trailers on both commits. That only applies if an agent made them.

Add uint_passthrough(_noconvert) and run the existing int conversion
matrix against it, which also pins __index__ taking precedence over
__int__ for unsigned targets. Gate the explicit PyNumber_Index call on
PYBIND11_INDEX_CHECK so objects without __index__ no longer raise and
clear an extra TypeError in convert mode, and select the PyPy/unsigned
condition with a constexpr bool instead of two #if'd if-heads.

Assisted-by: ClaudeCode:claude-fable-5-1
@henryiii

henryiii commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

I don't think we support PyPy < 7.3.16, so we could drop the branch.

henryiii and others added 2 commits October 8, 2026 00:51
PyLong_AsUnsignedLong does not call __int__, so the unsigned caster
falls back to PyNumber_Long and does not warn.

Assisted-by: ClaudeCode:claude-opus-5-5
PyPy below the supported 7.3.17 is not a target, and supported PyPy calls __index__ in PyLong_AsLong[Long] like CPython, so only unsigned targets need the explicit PyNumber_Index call. Also drops the complex.h comment that pointed at the removed branch.

Assisted-by: ClaudeCode:claude-opus-5-5
@quinncheong

Copy link
Copy Markdown
Author

The PyPy-only block in the integer caster is gone; only unsigned targets call PyNumber_Index now.

This branch has not been deployed

No deployments
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.

[BUG]: enum __eq__ returns False against numpy integers and other __index__ objects when the enum's underlying type is unsigned (regression in 3.1.0)

2 participants