Allow nanobind methods on non-nanobind classes. #104
Conversation
|
Sigh — pypy fails again. I'm not sure what's going on here (I'll have to figure out how to install the very new PyPy you're requiring). I'll look into it, but assuming I can fix that, would you object to this change? |
The problem turned out to be a pre-existing bug in the dispatch of bound methods. When using the non- |
There was a problem hiding this comment.
This looks like a nice improvement! One potential issue could arise when applying a constructor (__init__)-style function that has is_constructor set to true. In this case, the implementation would attempt to write into the PyObject * that has a different memory layout.
See: https://github.com/wjakob/nanobind/blob/master/src/nb_func.cpp#L637 and https://github.com/wjakob/nanobind/blob/master/src/nb_func.cpp#L766
My suggestion would be that is_constructor is set to false by default and only filled to its current value once we enter the branch for classes with the nb_type metaclass.
| if (!args_tmp) | ||
| return PyErr_NoMemory(); | ||
| args_tmp[0] = mb->self; | ||
| for (size_t i = 0; i < nargs; ++i) | ||
| for (size_t i = 0; i < nargs + nkwargs_in; ++i) |
This allows us to extend an existing Python (or pybind11!) class with methods defined in nanobind. This is handy if you want to add a small amount of nanobind code to a class defined in other ways. For example, one use is to migrate methods of a pybind11-defined class to nanobind one by one. All we have to do to allow this is remove the type check. I can't see anything that goes wrong if we simply allow dispatch to proceed.
Done! |
|
Thanks! |
This commit makes it possible to to retroactively add nanobind method bindings
to an existing Python class (previously, nanobind would fail with an error
message):
For example, this could look as follows in Python:
where
my_method_implmight be defined in C++This feature can particularly useful when
MyClasswas created using anotherbinding tool like pybind11. In this case, methods can be individually migrated
from one binding tool to the other over time.