Skip to content

Pure virtual overloaded functions #2351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
skgbanga opened this issue Jul 31, 2020 · 8 comments
Closed

Pure virtual overloaded functions #2351

skgbanga opened this issue Jul 31, 2020 · 8 comments

Comments

@skgbanga
Copy link

skgbanga commented Jul 31, 2020

Note that this question combines two sections in the documentation:

Consider this simple c++ class:

struct Loaded {
  void func(int) { std::cout << "int\n"; }
  void func(double) { std::cout << "double\n"; }
};

As explained in the documentation, one can create python bindings for this via:

PYBIND11_MODULE(pyfoo, m) {
  py::class_<Loaded>(m, "Loaded")
      .def(py::init<>())
      .def("func", py::overload_cast<int>(&Loaded::func))
      .def("func", py::overload_cast<double>(&Loaded::func));

And everything works perfectly on the python side.

>>> loaded = pyfoo.Loaded()
>>> loaded.func(10)
int
>>> loaded.func(10.0)
double

(It is a bit weird that one can do overloading in python via this).

Now consider another class:

class Listener {
 public:
  virtual ~Listener() = default;

  virtual void func(int) = 0;
  virtual void func(double) = 0;
};

As explained in the documentation, this requires us to setup a trampoline like this:

class PyListener : public Listener {
 public:
  void func(int i) override {
    PYBIND11_OVERLOAD_PURE(void, Listener, func, i);
  }
  void func(double d) override {
    PYBIND11_OVERLOAD_PURE(void, Listener, func, d);
  }
};

PYBIND11_MODULE(pyfoo, m) {
  py::class_<Listener, PyListener>(m, "Listener")
      .def(py::init<>())
      .def("func", py::overload_cast<int>(&Listener::func))
      .def("func", py::overload_cast<double>(&Listener::func));
}

Now, my question is: How does one override these methods on the python side. Python clearly doesn't support overloading, so how can I write my own class in this case.

Failed attempt:

>>> import pyfoo
>>> class Listener(pyfoo.Listener):
...     def func(self, n: int):
...             print("int")
...     def func(self, n: float):
...             print("float")
...
>>> l = Listener()
>>> l.func(10)
float
@YannickJadoul
Copy link
Collaborator

This is a Python and not a pybin11 issue. Python doesn't do overloading. The second func overwrites and replaces the first one (annotations don't dó anything in Python).

Write a single func in Python and check the type of n:

class Listener(pyfoo.Listener):
    def func(self, n):
        print("int" if isinstance(n, int) else "float" if isinstance(n, float) else "unknown")

@skgbanga
Copy link
Author

skgbanga commented Aug 3, 2020

@YannickJadoul Thanks for the response.

Right, I got that part. I am actually very curious about how:

>>> loaded = pyfoo.Loaded()
>>> loaded.func(10)
int
>>> loaded.func(10.0)
double

pybind is able to achieve overloading in the above case. Any pointers to documentation/code would be very helpful.

This is a Python and not a pybin11 issue.

I understand. So the c++ 'pattern' like this:

class Listener {
 public:
  virtual ~Listener() = default;

  virtual void func(int) = 0;
  virtual void func(double) = 0;
};

There really is no good alternative on python side. (apart from doing isinstance checks in a single function)

@YannickJadoul
Copy link
Collaborator

YannickJadoul commented Aug 3, 2020

pybind is able to achieve overloading in the above case. Any pointers to documentation/code would be very helpful.

A high-level description is here: https://pybind11.readthedocs.io/en/stable/advanced/functions.html#overload-resolution-order. The actual implementation is not explicitly documented, but that's because it's implementation, I suppose. You - most likely - very much don't want to look at the cpp_function::dispatcher code ;-)

But yes, pybind11 just accepts *args and **kwargs and does some rudimentary overload resolution to bridge between Python and C++.

There really is no good alternative on python side. (apart from doing isinstance checks in a single function)

Nope. Python is dynamically typed and doesn't do overload resolution on type arguments. So either implement your own overload resolution (in principle, it should be simple enough in this case?), or make the C++ interface you expose more pythonic by just giving it separate names, I'd say.

@skgbanga
Copy link
Author

skgbanga commented Aug 3, 2020

very much don't want to look at the cpp_function::dispatcher code

Thanks for sharing this code! I actually never looked at implementation, and it seems the core components in dispatcher are fairly well documented.

(in principle, it should be simple enough in this case?)

In this case, yes. But I have a listener class which has more than 20 overloads for different types.

make the C++ interface you expose more pythonic by just giving it separate names, I'd say.

I am going to provide a pythonic shim over the c++ interface.

Thanks for your help.

@YannickJadoul
Copy link
Collaborator

(in principle, it should be simple enough in this case?)

In this case, yes. But I have a listener class which has more than 20 overloads for different types.

I see. I'd just suggest to think how you would design the interface in Python, if there wasn't a C++ implementation. pybind11 is often flexible enough to allow you to wrap the C++ interface and provide that Python interface.

@skgbanga
Copy link
Author

skgbanga commented Aug 3, 2020

@YannickJadoul I followed your advice and am running into an issue. Here are the details:

This is a third party code that I have no control over:

class Listener {
 public:
  virtual ~Listener() = default;

  virtual void func(int) = 0;
  virtual void func(double) = 0;
};

void invoke(Listener* listener) {
  listener->func(42);
}

To make it work nicely with python, I added the following:

class MyListener : public Listener {
 public:
  virtual ~MyListener() = default;

  void func(int i) override { funci(i); }
  void func(double d) override { funcd(d); };

  // inject new interface
  virtual void funci(int) = 0;
  virtual void funcd(double) = 0;
};

class PyListener : public MyListener {
 public:
  void funci(int i) override {
    PYBIND11_OVERLOAD_PURE(void, MyListener, funci, i);
  }
  void funcd(double d) override {
    PYBIND11_OVERLOAD_PURE(void, MyListener, funcd, d);
  }
};

PYBIND11_MODULE(pyfoo, m) {
  py::class_<MyListener, PyListener>(m, "Listener")
      .def(py::init<>())
      .def("funci", &MyListener::funci)
      .def("funcd", &MyListener::funcd);

  m.def("invoke", &invoke);
}

On the python side, I am doing the following:

import pyfoo

class A(pyfoo.Listener):
    def funci(self, a):
        print("answer is {}".format(a))

    def funcd(self, d):
        print("double answer is {}".format(d))

 
a = A()
pyfoo.invoke(a)

And I get the following error:

(venv:zen) ~/workspace/dev/py-zen$ python -i test.py
Traceback (most recent call last):
  File "test.py", line 19, in <module>
    pyfoo.invoke(a)
TypeError: invoke(): incompatible function arguments. The following argument types are supported:                                                                            
    1. (arg0: Listener) -> None

Invoked with: <__main__.A object at 0x7f108de538e0>

My guess is that I need to make pybind aware that my A object is actually a Listener such that it can properly upcast my A to a pointer of Listener, but I haven't been able to figure that out yet from the documentation.

Once again, thanks for your help.

@skgbanga
Copy link
Author

skgbanga commented Aug 4, 2020

ok. I think I have stumbled upon a working solution.
My first attempt at making it aware of the base class was this:

  py::class_<MyListener, PyListener, Listener>(m, "Listener")
      .def(py::init<>())
      .def("funci", &MyListener::funci)
      .def("funcd", &MyListener::funcd);

However, this gave the following error when importing my module:

Traceback (most recent call last):
  File "test.py", line 7, in <module>
    import pyfoo
ImportError: generic_type: type "Listener" referenced unknown base type "Listener"

This was interesting. It meant that python is not able to find a previous Listener class. So I added one in module:

PYBIND11_MODULE(pyfoo, m) {
  py::class_<Listener>(m, "Listener");
  py::class_<MyListener, PyListener, Listener>(m, "MyListener")
      .def(py::init<>())
      .def("funci", &MyListener::funci)
      .def("funcd", &MyListener::funcd);

And this seemed to have done the trick.

I don't fully understand the template arguments of the class_ template class. e.g. the inheritance hierarchy is:

Listener -> MyListener -> PyListener

However, in the template arguments, the order we have passed is a bit bizzare.

Looking at the code, it seems that both base and subtypes can be passed as template args (thought I don't understand the code yet)

      template <typename T> using is_subtype = detail::is_strict_base_of<type_, T>;
      template <typename T> using is_base = detail::is_strict_base_of<T, type_>;

Anyway, I have a working solution now. Thanks for your help on this.

@YannickJadoul
Copy link
Collaborator

@skgbanga This should also work, I believe?

class Listener {
 public:
  virtual ~Listener() = default;

  virtual void func(int) = 0;
  virtual void func(double) = 0;
};

void invoke(Listener* listener) {
  listener->func(42);
}

class PyListener : public Listener {
 public:
  void func(int i) override {
    PYBIND11_OVERLOAD_PURE_NAME(void, Listener, funci, func, i);
  }
  void func(double d) override {
    PYBIND11_OVERLOAD_PURE_NAME(void, Listener, funcd, func, d);
  }
};

PYBIND11_MODULE(pyfoo, m) {
  py::class_<Listener, PyListener>(m, "Listener")
      .def(py::init<>())
      .def("funci", py::overload_cast<int>(&Listener::func))
      .def("funcd", py::overload_cast<double>(&Listener::func));

  m.def("invoke", &invoke);
}

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

No branches or pull requests

2 participants