casting c++-base class to py-derived class failes

TillLeiden commented 5 years ago

Issue description

Hello, Consider a derived py-class (PClass) with a C++ class as base class (CClass). If I want cast a CClass-Object to a PClass-Object, python gives me the following error: TypeError: class assignment: 'PClass' deallocator differs from 'cpp_module.CClass'

I tested this with python3, clang-6.0, gcc 5.4, c++11 and c++17

Reproducible example code

#include <pybind11/pybind11.h>

class CClass {};

namespace py = pybind11;

PYBIND11_MODULE(cpp_module, m) {
   py::class_<CClass>(m, "CClass").def(py::init<>());
}

working example with python

class PClassA():
    def __init__(self):
        pass

class PClassB(PClassA):
    def __init__(self):
        super().__init__()

# py -> py example
a_class = PClassA()
b_class = PClassB()
a_class.__class__ = PClassB # this works

failing example with C++

from cpp_module import CClass

class PClass(CClass):
    def __init__(self):
        super().__init__()

# c++ -> py example
c_class = CClass()
p_class = PClass()
c_class.__class__ = PClass  # this failes

output

Traceback (most recent call last):
  File "test.py", line 25, in <module>
    c_class.__class__ = PClass  # this failes
TypeError: __class__ assignment: 'PClass' deallocator differs from 'cpp_module.CClass'

mmodenesi commented 5 years ago

Hi!

a_class.__class__ == PClassB # this works

you are comparing

c_class.__class__ = PClass # this failes

You are assigning

is this a typo?

TillLeiden commented 5 years ago

Hi,

yes that was a typo, sorry for that.

eacousineau commented 5 years ago

Howdy @TillLeiden!

Since you want to have a Python class inherit from a C++ base class, there might be a few things:

Your examples still have obj.__class__ = something; can you update this code and re-run your example to ensure you're checking the actual behavior you want?
You might need to use a trampoline class as mentioned in the docs; I've messed around with these internals before on a fork, but I don't remember the exact magic for ensuring Python child classes of C++ base classes works.
At the end of this section in the docs, there is a warning to not use super, but instead use an explicit Base.__init__(self, ...).

henryiii commented 4 years ago

The idea here is that one would like to "enhance" a C++ class in Python, then take produced C++ classes and cast them to the enhanced Python classes. So you want the assignment, not a check.

For example, your c++ code produces an instance of CClass. You want to give a PClass to the user, without making a copy of the underlying class.

I assume this is a clash related to pybind11_builtins.pybind11_type?

henryiii commented 4 years ago

@TillLeiden, you should get the same behavior if you remove all the Python inits and just put pass in the classes.

from cpp_module import CClass

# Python enhanced version
class PClass(CClass):
    pass

c_class = CClass()
c_class.__class__ = PClass  # this fails

eacousineau commented 4 years ago

@henryiii Per convo on Gitter, it sounds like you also want to be able to extend it to multiple classes on-the-fly? Is this effectively the contract you'd want to maintain? (This is a slightly less trivial extension of the working Python example above) https://github.com/eacousineau/repro/blob/c3ab19fc94c85473a4fd9c07d65ab9f36362db82/python/py_class_swap.py

But yeah, understanding this better, yes, it is a class with pybind11_type. Effectively, there are unique codepaths that pybind11 wants to take with its meta type when it is dealing with Python-subclasses of bound C++ classes.

Since this the reassignment is done after construction time, there's now a discrepancy between the recorded type metadata (see type_record in attr.h) and the now-disagreeing updated type metadata.

Let me see if making it an actual trampoline resolves the error about deallocator mismatch (which looks like it's CPython, not pybind).

eacousineau commented 4 years ago

Yeah, making the base class a trampoline does not work with the hot-swapping: https://github.com/eacousineau/repro/blob/ee4b03a98a448d947de469366c511ec49bbf77d6/python/pybind11/custom_tests/test_tmp.cc#L81-L82

So yeah, if you want something like this to really work with casting, these are the only three routes I can think of at the moment:

Route 1 - Modify pybind source to fix the dealloc error

This won't be fun, but may be fruitful if it doesn't mess up anything else?

Route 2 - Explicitly Wrap / Unwrap

Use something like wrapt to just wrap the object and supply your augmentation, rather than tinkering with inheritance. Here's an example where I try to mimick C++-const views into an object (see #717 for more info): https://github.com/RobotLocomotion/drake/blob/297063a3eca26fe9fb47fcc06a51762180bb435a/bindings/pydrake/common/cpp_const.py#L140

If your types have a very specific trait, e.g. they inherit from something or whatevs that can be constexpr evaluated, then you could specialize type_caster to handle your wrapping / unwrapping. (If this is the case, I'd strongly recommend going this way.)

If your types do not have a specific trait, you will need to either shadow the specialization that calls into type_caster_generic, and inject your wrapping / unwrapping logic there: https://github.com/pybind/pybind11/blob/bd24155b8bf798f9f7022b39acd6d92e52d642d6/include/pybind11/cast.h#L936 Or (more suggested) explicitly wrap your types / casters. I had done so here: https://github.com/RobotLocomotion/drake/issues/7793#issuecomment-359992066

Route 3 - Automagically Wrap / Unwrap by Modifying pybind source

Modify the pybind11 source code to handle your unwrapping, maybe trying to make it somewhat of a generic interface; e.g. something like:

Add a constexpr trait so it doesn't slow down nominal classes, and
Has some function, like _underlying_pybind_object.

Then you can modify type_caster_generic to check hasattr(obj, "_underlying_pybind_object"), and recover the wrapped object. However, if code is sensitive to round-trip casting (e.g. pass throughs), you may lose your wrapping unless you have some attribute on the embedded object saying something like "hey, I have a weakref to my augmented wrapper".

Routes 1 and 2 (if your classes have specific traits) sound somewhat feasible. Otherwise, seems less so...

pybind / pybind11