pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.18k stars 2.06k forks source link

[FEATURE REQUEST]: Dynamic class names (`pybind11::class_` constructor taking `std::string`) #4777

Open burnpanck opened 11 months ago

burnpanck commented 11 months ago

Required prerequisites

What version (or hash if on master) of pybind11 are you using?

2.10.4

Problem description

I often find myself wrapping C++ templates using pybind11. In this case, I can write a template function that instantiates and registers a specific form of the template. Of course, if I do this for multiple instantiations, I end up with multiple python classes, and those should have distinct names. I would like to generate these names in the template function at runtime. The problem is that the class_ constructors only take const char * for the name, providing no way of managing the dynamic memory where the runtime-generated name resides (a naive use of std::string::c_str() obviously leads to dangling references).

Python's type system probably wasn't designed with C++ templates in mind, so it likely doesn't provide an direct way of resource management for the name of a type object. However, we can make a workaround using weak references, following the example given in the pybind11 documentation. I have been using the following:

template <typename Cls, typename... Options, typename... Args>
auto make_class(py::module_ &m, std::string name, Args &&...args) {
  auto holder = std::make_unique<std::string>(std::move(name));
  auto ret = py::class_<Cls, Options...>(m, holder->c_str(), std::forward<Args>(args)...);
  py::cpp_function cleanup_callback([holder = std::move(holder)](py::handle weakref) mutable {
    holder.reset();
    weakref.dec_ref();
  });
  (void)pybind11::weakref(ret, cleanup_callback).release();
  return ret;
}

I wonder if something like this could be reasonably included in a dedicated constructor?

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

Not a regression