Expand float and complex strict mode to allow ints and ints/float (for PEP 484 compatibility).#5879
Conversation
|
I have started working on this too. |
This is known at compile time so it can be constexpr
|
Here is my implementation and documentation changes. It needs tests but I am going to stop for today. |
|
You will need to remove the constexpr check. I didn't realise it was a newer C++ feature |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
TODO: figure out why it doesn't work in C++11. |
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
|
I have just found that a python float passed to an int argument will raise a TypeError. This is probably something we should resolve here because it effects these tests. m.def("func", [](int){});>>> func.__doc__
func(arg: typing.SupportsInt) -> None
>>> func(5.5)
TypeError
>>> func(numpy.float32(5.5)) # This works fine |
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.
|
Here is my proposed change. You will need to update the tests. Here are my unittests |
|
I noticed something else to have address. According to the documentation all of int,float,complex support falling back on |
/__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 {
| ^
gentlegiantJGC
left a comment
There was a problem hiding this comment.
I can't see any obvious issues
|
I will open a new issue about the custom float -> int conversion |
…eturning 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
|
The extra tests (commit f1d8158) are:
This is a bit on the paranoid side, but I wanted to reassure myself that we're not putting the world through a behavior change without having looked at every dark corner. |
|
BTW, having looked at this so intensely ... bool py_err = py_value == (py_type) -1 && PyErr_Occurred();this will be OK if Python is healthy, but if something goes wrong (e.g. UB, or bug in Python beta testing) and So that condition basically errs on the unsafe side, it prioritizes the return value check over error detection. I think it'd be better to write: bool py_err = PyErr_Occurred();
if (py_err) assert(py_value == (py_type) -1);This prioritizes error detection over return value validation. WDYT? |
…ere (e.g. UB, or bug in Python beta testing). See also: pybind#5879 (comment)
Same. I looked through everything several times now. Thanks! |
|
FYI — This PR is included in PR #5988 |
As of pybind/pybind11#5879, pybind11 will convert Python `int` to C `float`/`double`. For `std::variant`, pybind11 will attempt to convert arguments in the order of the type. So if `double` occurs earlier in the variant, then `int`/`long` will never be produced. By putting `int`/`long` before `double`, pybind11 will attempt that conversion first and we'll continue to produce our deprecation warning correctly. Fixes matplotlib#31495
As of pybind/pybind11#5879, pybind11 will convert Python `int` to C `float`/`double`. For `std::variant`, pybind11 will attempt to convert arguments in the order of the type. So if `double` occurs earlier in the variant, then `int`/`long` will never be produced. By putting `int`/`long` before `double`, pybind11 will attempt that conversion first and we'll continue to produce our deprecation warning correctly. Fixes #31495
Description
Breaking change to allow Python ints into float when under strict typing (
noconvert), to better match PEP 484 numeric tower rules.This change was also done to complex to allow floats and ints. Now that Python ints can be passed into floats it changes behavior for overload resolution. A method that takes float that is registered before a method that takes an int will now get executed when a Python int is passed in. This overload resolution also affects methods with
std::complex.Resolves #5878
Provides a work around for #5767
NOTE: This PR was reviewed extensively and is approved. However, because of the breaking change, it will be merged only after the next patch release.
Suggested changelog entry:
Breaking change for PEP 484 compatibility: Expand float and complex strict mode to allow ints and ints/float.
📚 Documentation preview 📚: https://pybind11--5879.org.readthedocs.build/