-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Open
Description
(on 2.4.3)
Looks like there's a mix of a few issues in the example below:
- First, least precision floats would be picked if you don't carefully order your overloads (as @ax3l mentioned in Float Overloads Specialize to Least Precision #1512), sounds like that's something that may be done automatically
- There's no builtin float32, so both floats and doubles show up as 'float' on Python side; this would have been fine unless...
- ... passing
np.float32()actually worked. You may expect it would have worked but because it doesn't inherit frombuiltin.floatit won't. Is it worth special-casing scalar float32 case?
I do sort of understand where this is coming from, but someone who doesn't know py11 internals may assume that "well, np.float32[:] arrays work, and np.float64[:] arrays work, and I can pass np.float64 and builtin.float for double-type arguments, so np.float32 should work as well?", but then it gets converted to a different type altogether.
#include <pybind11/pybind11.h>
namespace py = pybind11;
auto f_double(double x) { return "double"; }
auto f_float(float x) { return "float"; }
auto f_int(int x) { return "int"; }
PYBIND11_MODULE(py11, m) {
// double/float: these two overloads have to be ordered this way... (see #1512)
m.def("f", &f_double, py::arg("x"));
m.def("f", &f_float, py::arg("x"));
// int
m.def("f", &f_int, py::arg("x"));
}Let's give this a spin:
>>> f?
Overloaded function.
1. f(x: float) -> str # <-- ?
2. f(x: float) -> str
3. f(x: int) -> str
>>> f(1)
'int' # ok
>>> f(1.)
'double' # note: would have been float if two overloads were swapped
>>> f(np.float64(1.))
'double' # so far so good
>>> f(np.float32(1.))
<ipython-input-7-4d9dada8d141>:1: DeprecationWarning: an integer is required (got type numpy.float32). Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python.
f(np.float32(1.))
'int' # not so goodax3l and yosh-matsuda
Metadata
Metadata
Assignees
Labels
No labels