Skip to content

Add typing.SupportsIndex to int/float/complex type hints#5891

Merged
rwgk merged 6 commits intopybind:masterfrom
rwgk:add_missing_SupportsIndex
Nov 11, 2025
Merged

Add typing.SupportsIndex to int/float/complex type hints#5891
rwgk merged 6 commits intopybind:masterfrom
rwgk:add_missing_SupportsIndex

Conversation

@rwgk
Copy link
Collaborator

@rwgk rwgk commented Nov 10, 2025

Description

Production code changes isolated from #5879

Quoting from there:

This corrects a mistake where these types were supported but the type hint was not updated to reflect that SupportsIndex objects are accepted.

Isolated out mainly to have the corresponding sprawling changes to the unit tests separately, to minimize the chances that we're overlooking something important in the more critical PR #5879.

Note: In contrast to #5879, the noconvert conversion behavior is unchanged. Only the complex-caster convert behavior is changed (bug fix), and the type hints.

Note: Most of the legwork here was done by Cursor.

@gentlegiantJGC @InvincibleRMC for visibility

Suggested changelog entry:

  • Correct a mistake where support for __index__ was added, but the type hints were not updated to reflect that SupportsIndex objects are accepted. Also fix a long-standing bug: the complex-caster was not accepting __index__ in convert mode.

rwgk added 3 commits November 10, 2025 12:23
This corrects a mistake where these types were supported but the type
hint was not updated to reflect that SupportsIndex objects are accepted.

To track the resulting test failures:

The output of

"$(cat PYROOT)"/bin/python3 $HOME/clone/pybind11_scons/run_tests.py $HOME/forked/pybind11 -v

is in

~/logs/pybind11_pr5879_scons_run_tests_v_log_2025-11-10+122217.txt
@henryiii
Copy link
Collaborator

henryiii commented Nov 10, 2025

Only int SupportsIndex. float and complex do not? SupportsIndex means a type can be losslessly converted to an integer for indexing, that is something[x] is supported for x. They should not have an __index__. Only int and types like int should.

Copy link
Contributor

@InvincibleRMC InvincibleRMC left a comment

Choose a reason for hiding this comment

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

You're missing the tests I believe which show this is already happening at run time.

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 10, 2025

Only int SupportsIndex. float and complex do not? SupportsIndex means a type can be losslessly converted to an integer for indexing, that is something[x] is supported for x. They should not have an __index__. Only int and types like int should.

I'm not completely clear about that (yet) myself.

The goal of this PR

  • is not to reach a final desired state,
  • but to correct a particular mistake in isolation, to make other changes to the tests easier to review.

Currently I'm thinking PR #5879 is good as-is, but

  • I don't want to make such a big leap,
  • and I think we should release pybind11 v3.0.2 without the (other) production code changes in PR 5879,
  • but right after the release merge the rest of PR 5879.

@InvincibleRMC
Copy link
Contributor

Only int SupportsIndex. float and complex do not? SupportsIndex means a type can be losslessly converted to an integer for indexing, that is something[x] is supported for x. They should not have an __index__. Only int and types like int should.

If you look at the bottom of float and complex they fallback on index. https://docs.python.org/3.13/library/functions.html#float

@henryiii
Copy link
Collaborator

Ahh, this is for input only?

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 10, 2025

Ahh, this is for input only?

I want to actually merge this PR, then merge master into PR 5879. Hopefully that'll clear up a lot of the unit test diffs there.

@rwgk rwgk marked this pull request as ready for review November 10, 2025 22:03
@rwgk
Copy link
Collaborator Author

rwgk commented Nov 10, 2025

oops, I clicked ready for review button too fast, I'm seeing the request for change only now

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 10, 2025

@InvincibleRMC wrote:

You're missing the tests I believe which show this is already happening at run time.

Could you please help me with concrete pointers?

I did not try to transfer any additional tests from PR 5879, but if you point at specific ones that you think should be included here, I could do that. I could also give you write permission to this branch, so that you can make changes directly. Please let me know.

@InvincibleRMC
Copy link
Contributor

InvincibleRMC commented Nov 10, 2025

The tests that pass in custom classes with __index__ into float and complex args. I think it's float convert and complex convert.

rwgk added 2 commits November 10, 2025 14:41
Summary:

  Changes Made

  1. **C++ Bindings** (`tests/test_builtin_casters.cpp`)

  • Added complex_convert and complex_noconvert functions needed for the tests

  2. **Python Tests** (`tests/test_builtin_casters.py`)

  `test_float_convert`:
  • Added Index class with __index__ returning -7
  • Added Int class with __int__ returning -5
  • Added test showing Index() works with convert mode: assert pytest.approx(convert(Index())) == -7.0
  • Added test showing Index() doesn't work with noconvert mode: requires_conversion(Index())
  • Added additional assertions for int literals and Int() class

  `test_complex_cast`:
  • Expanded the test to include convert and noconvert functionality
  • Added Index, Complex, Float, and Int classes
  • Added test showing Index() works with convert mode: assert convert(Index()) == 1 and assert isinstance(convert(Index()), complex)
  • Added test showing Index() doesn't work with noconvert mode: requires_conversion(Index())
  • Added type hint assertions matching the SupportsIndex additions

  These tests demonstrate that custom __index__ objects work with float and complex in convert mode, matching the typing.SupportsIndex type hint added in PR
  5891.
@rwgk
Copy link
Collaborator Author

rwgk commented Nov 10, 2025

The tests that pass in custom classes with index into float and complex args. I think it's float convert and complex convert.

@InvincibleRMC do these commits look right?

  • 77c7fca — Cursor-generated commit: Added the Index() tests from PR 5879.
  • 9c333f1 — Reflect behavior changes going back from PR 5879 to master. This diff will have to be reapplied under PR 5879.

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 11, 2025

The three pypy-3.10 jobs (ubuntu, macos-13, windows-latest) are failing with the error below.

Fixing that will be a behavior change, but I think we should do that in this PR, because it's in convert mode, i.e. "related but different" from the goal of PR 5879. It's more a bug fix actually IMO than a behavior change.

I.e. we'd need to extract out the corresponding changes from PR 5879 as well.

@InvincibleRMC could you please let me know if you agree or disagree?

I'll play with Cursor some more to see if it is capable of doing that for us.

=================================== FAILURES ===================================
______________________________ test_complex_cast _______________________________

doc = <conftest.SanitizedString object at 0x0000000011743210>

    def test_complex_cast(doc):
        """std::complex casts"""
    
        class Complex:
            def __complex__(self) -> complex:
                return complex(5, 4)
    
        class Float:
            def __float__(self) -> float:
                return 5.0
    
        class Int:
            def __int__(self) -> int:
                return 3
    
        class Index:
            def __index__(self) -> int:
                return 1
    
        assert m.complex_cast(1) == "1.0"
        assert m.complex_cast(1.0) == "1.0"
        assert m.complex_cast(Complex()) == "(5.0, 4.0)"
        assert m.complex_cast(2j) == "(0.0, 2.0)"
    
        convert, noconvert = m.complex_convert, m.complex_noconvert
    
        def requires_conversion(v):
            pytest.raises(TypeError, noconvert, v)
    
        def cant_convert(v):
            pytest.raises(TypeError, convert, v)
    
        assert (
            doc(convert)
            == "complex_convert(arg0: typing.SupportsComplex | typing.SupportsFloat | typing.SupportsIndex) -> complex"
        )
        assert doc(noconvert) == "complex_noconvert(arg0: complex) -> complex"
    
        assert convert(1) == 1.0
        assert convert(2.0) == 2.0
        assert convert(1 + 5j) == 1.0 + 5.0j
        assert convert(Complex()) == 5.0 + 4j
        assert convert(Float()) == 5.0
        assert isinstance(convert(Float()), complex)
        cant_convert(Int())
>       assert convert(Index()) == 1
E       TypeError: complex_convert(): incompatible function arguments. The following argument types are supported:
E           1. (arg0: typing.SupportsComplex | typing.SupportsFloat | typing.SupportsIndex) -> complex
E       
E       Invoked with: <test_builtin_casters.test_complex_cast.<locals>.Index object at 0x000000001170bec0>

Complex    = <class 'test_builtin_casters.test_complex_cast.<locals>.Complex'>
Float      = <class 'test_builtin_casters.test_complex_cast.<locals>.Float'>
Index      = <class 'test_builtin_casters.test_complex_cast.<locals>.Index'>
Int        = <class 'test_builtin_casters.test_complex_cast.<locals>.Int'>
cant_convert = <function test_complex_cast.<locals>.cant_convert at 0x00007f349fb85ce0>
convert    = <builtin_function_or_method object at 0x000000000d88aa60>
doc        = <conftest.SanitizedString object at 0x0000000011743210>
noconvert  = <builtin_function_or_method object at 0x000000000d88aaa0>
requires_conversion = <function test_complex_cast.<locals>.requires_conversion at 0x00007f349fb85c40>

../../tests/test_builtin_casters.py:529: TypeError

@InvincibleRMC
Copy link
Contributor

Ah yeah that makes sense. Copying the back porting of __index__ to old versions of pypy.

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 11, 2025

OK, I'll get Cursor going on this.

(Peeling the PR apart like this is a great way for me to get my head around it. I wasn't able to follow the individual commits on 5879; I'm operating on a tiny time budget.)

Extract PyPy-specific __index__ backporting from PR 5879 to fix PyPy 3.10
test failures in PR 5891. This adds:

1. PYBIND11_INDEX_CHECK macro in detail/common.h:
   - Uses PyIndex_Check on CPython
   - Uses hasattr check on PyPy (workaround for PyPy 7.3.3 behavior)

2. PyPy-specific __index__ handling in complex.h:
   - Handles __index__ objects on PyPy 7.3.7's 3.8 which doesn't
     implement PyLong_*'s __index__ calls
   - Mirrors the logic used in numeric_caster for ints and floats

This backports __index__ handling for PyPy, matching the approach
used in PR 5879's expand-float-strict branch.
@rwgk
Copy link
Collaborator Author

rwgk commented Nov 11, 2025

After commit 6267614 all tests passed. Merging. Thanks @InvincibleRMC!

I'll try to use Cursor to update 5879, hoping it'll be able to deal with any merge conflicts.

@rwgk rwgk merged commit 9f1187f into pybind:master Nov 11, 2025
147 of 148 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Nov 11, 2025
@rwgk rwgk deleted the add_missing_SupportsIndex branch November 11, 2025 04:27
rwgk added a commit to InvincibleRMC/pybind11 that referenced this pull request Nov 12, 2025
During merge, HEAD's regex pattern was kept, but master's version is preferred.
The order of ` ` and `\|` in the character class is arbitrary. Keep master's order
(already fixed in PR pybind#5891; sorry I missed looking back here when working on 5891).
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Jan 21, 2026
rwgk added a commit that referenced this pull request Feb 17, 2026
…r PEP 484 compatibility). (#5879)

* init

Signed-off-by: Michael Carlstrom <[email protected]>

* Add constexpr to is_floating_point check

This is known at compile time so it can be constexpr

* Allow noconvert float to accept int

* Update noconvert documentation

* Allow noconvert complex to accept int and float

* Add complex strict test

* style: pre-commit fixes

* Update unit tests so int, becomes double.

* style: pre-commit fixes

* remove if (constexpr)

Signed-off-by: Michael Carlstrom <[email protected]>

* fix spelling error

* bump order in #else

* Switch order in c++11 only section

Signed-off-by: Michael Carlstrom <[email protected]>

* ci: trigger build

* ci: trigger build

* Allow casting from float to int

The int type caster allows anything that implements __int__ with explicit exception of the python float. I can't see any reason for this.
This modifies the int casting behaviour to accept a float.
If the argument is marked as noconvert() it will only accept int.

* tests for py::float into int

* Update complex_cast tests

* Add SupportsIndex to int and float

* style: pre-commit fixes

* fix assert

* Update docs to mention other conversions

* fix pypy __index__ problems

* style: pre-commit fixes

* extract out PyLong_AsLong __index__ deprecation

Signed-off-by: Michael Carlstrom <[email protected]>

* style: pre-commit fixes

* Add back env.deprecated_call

Signed-off-by: Michael Carlstrom <[email protected]>

* remove note

Signed-off-by: Michael Carlstrom <[email protected]>

* remove untrue comment

Signed-off-by: Michael Carlstrom <[email protected]>

* fix noconvert_args

Signed-off-by: Michael Carlstrom <[email protected]>

* resolve error

Signed-off-by: Michael Carlstrom <[email protected]>

* Add comment

Signed-off-by: Michael Carlstrom <[email protected]>

* [skip ci]

tests: Add overload resolution test for float/int breaking change

Add test_overload_resolution_float_int() to explicitly test the breaking
change where int arguments now match float overloads when registered first.

The existing tests verify conversion behavior (int -> float, int/float -> complex)
but do not test overload resolution when both float and int overloads exist.
This test fills that gap by:

- Testing that float overload registered before int overload matches int(42)
- Testing strict mode (noconvert) overload resolution breaking change
- Testing complex overload resolution with int/float/complex overloads
- Documenting the breaking change explicitly

This complements existing tests which verify 'can it convert?' by testing
'which overload wins when multiple can convert?'

* Add test to verify that custom __index__ objects (not PyLong) work correctly with complex conversion. These should be consistent across CPython, PyPy, and GraalPy.

* Improve comment clarity for PyPy __index__ handling

Replace cryptic 'So: PYBIND11_INDEX_CHECK(src.ptr())' comment with
clearer explanation of the logic:

- Explains that we need to call PyNumber_Index explicitly on PyPy
  for non-PyLong objects
- Clarifies the relationship to the outer condition: when convert
  is false, we only reach this point if PYBIND11_INDEX_CHECK passed
  above

This makes the code more maintainable and easier to understand
during review.

* Undo inconsequential change to regex in test_enum.py

During merge, HEAD's regex pattern was kept, but master's version is preferred.
The order of ` ` and `\|` in the character class is arbitrary. Keep master's order
(already fixed in PR #5891; sorry I missed looking back here when working on 5891).

* test_methods_and_attributes.py: Restore existing `m.overload_order(1.1)` call and clearly explain the behavior change.

* Reject float → int conversion even in convert mode

Enabling implicit float → int conversion in convert mode causes
silent truncation (e.g., 1.9 → 1). This is dangerous because:

1. It's implicit - users don't expect truncation when calling functions
2. It's silent - no warning or error
3. It can hide bugs - precision loss is hard to detect

This change restores the explicit rejection of PyFloat_Check for integer
casters, even in convert mode. This is more in line with Python's behavior
where int(1.9) must be explicit.

Note that the int → float conversion in noconvert mode is preserved,
as that's a safe widening conversion.

* Revert test changes that sidestepped implicit float→int conversion

This reverts all test modifications that were made to accommodate
implicit float→int conversion in convert mode. With the production
code change that explicitly rejects float→int conversion even in
convert mode, these test workarounds are no longer needed.

Changes reverted:
- test_builtin_casters.py: Restored cant_convert(3.14159) and
  np.float32 conversion with deprecated_call wrapper
- test_custom_type_casters.py: Restored TypeError expectation for
  m.ints_preferred(4.0)
- test_methods_and_attributes.py: Restored TypeError expectation
  for m.overload_order(1.1)
- test_stl.py: Restored float literals (2.0) that were replaced with
  strings to avoid conversion
- test_factory_constructors.py: Restored original constructor calls
  that were modified to avoid float→int conversion

Also removes the unused avoid_PyLong_AsLong_deprecation fixture
and related TypeVar imports, as all uses were removed.

* Replace env.deprecated_call() with pytest.deprecated_call()

The env.deprecated_call() function was removed, but two test cases
still reference it. Replace with pytest.deprecated_call(), which is
the standard pytest context manager for handling deprecation warnings.

Since we already require pytest>=6 (see tests/requirements.txt), the
compatibility function is obsolete and pytest.deprecated_call() is
available.

* Update test expectations for swapped NoisyAlloc overloads

PR 5879 swapped the order of NoisyAlloc constructor overloads:
- (int i, double) is now placement new (comes first)
- (double d, double) is now factory pointer (comes second)

This swap is necessary because pybind11 tries overloads in order
until one matches. With int → float conversion now allowed:

- create_and_destroy(4, 0.5): Without the swap, (double d, double)
  would match first (since int → double conversion is allowed),
  bypassing the more specific (int i, double) overload. With the
  swap, (int i, double) matches first (exact match), which is
  correct.

- create_and_destroy(3.5, 4.5): (int i, double) fails (float → int
  is rejected), then (double d, double) matches, which is correct.

The swap ensures exact int matches are preferred over double matches
when an int is provided, which is the expected overload resolution
behavior.

Update the test expectations to match the new overload resolution
order.

* Resolve clang-tidy error:

/__w/pybind11/pybind11/include/pybind11/cast.h:253:46: error: repeated branch body in conditional chain [bugprone-branch-clone,-warnings-as-errors]
  253 |         } else if (PyFloat_Check(src.ptr())) {
      |                                              ^
/__w/pybind11/pybind11/include/pybind11/cast.h:258:10: note: end of the original
  258 |         } else if (convert || PYBIND11_LONG_CHECK(src.ptr()) || PYBIND11_INDEX_CHECK(src.ptr())) {
      |          ^
/__w/pybind11/pybind11/include/pybind11/cast.h:283:16: note: clone 1 starts here
  283 |         } else {
      |                ^

* Add test coverage for __index__ and __int__ edge cases: incorrectly returning float

These tests ensure that:
- Invalid return types (floats) are properly rejected
- The fallback from __index__ to __int__ works correctly in convert mode
- noconvert mode correctly prevents fallback when __index__ fails

* Minor comment-only changes: add PR number, for easy future reference

* Ensure we are not leaking a Python error is something is wrong elsewhere (e.g. UB, or bug in Python beta testing).

See also: #5879 (comment)

* [skip ci] Bump PYBIND11_INTERNALS_VERSION to 12 (for PRs 5879, 5887, 5960)

---------

Signed-off-by: Michael Carlstrom <[email protected]>
Co-authored-by: gentlegiantJGC <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
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.

3 participants