cpp-pybind11-playground icon indicating copy to clipboard operation
cpp-pybind11-playground copied to clipboard

access DocumentAPI / Document in ScripterAPI / Sample

Open aoloe opened this issue 8 years ago • 10 comments

when running

https://github.com/aoloe/cpp-pybind11-playground/blob/master/scripter-api/python/set-bar.py

print(Sample.a)
document = Sample.document
print(document.a)

i get this error message:

terminate called after throwing an instance of 'pybind11::error_already_set'
  what():  TypeError: Unable to convert function return value to a Python type! The signature was
        (self: scripterapi.Sample) -> ScripterAPI::Document

At:
  ../python/set-bar.py(7): <module>

the offending line is document = Sample.document.

the Sample and the Document classes for the API are defined here:

https://github.com/aoloe/cpp-pybind11-playground/tree/master/scripter-api/src/scripterAPI

aoloe avatar Sep 27 '17 17:09 aoloe

henriii says:

the code is creating two modules, but only making one. The PYBIND11_MODULE must have the same name as the .so, so you can’t put two of them in one .so

Here is how you’d make one module, but split up the definitions:

http://pybind11.readthedocs.io/en/master/faq.html#how-can-i-reduce-the-build-time

aoloe avatar Sep 27 '17 17:09 aoloe

You are doing something funny there. The PYBIND11_MODULE must be used exactly once for the entire module. No need for document.cpp (scratch it), and put everything inside scripterAPI.cpp:

PYBIND11_MODULE(scripterapi, m) {
    py::class_<ScripterAPI::ScripterAPI>(m, "Sample")
        .def(py::init<>())
        .def_readwrite("a", &ScripterAPI::ScripterAPI::a)
        .def_readwrite("document", &ScripterAPI::ScripterAPI::documentAPI)
        ;
    py::class_<ScripterAPI::Document>(m, "Document")
        .def(py::init<>())
        .def_readwrite("document", &ScripterAPI::Document::a)
        ;

}

That will work.

eudoxos avatar Sep 28 '17 12:09 eudoxos

hi thanks!

in this sample i'm also experimenting with a modular architecture... indeed, the code was wrong and the hint (and the link above) by henriii got me on the good path...

now i have a modular version that correctly compiles and loads! i'll push it this evening (i forgot to do so, yesterday evening).

and, of course, now i'm lost in the next issue: how to share the "document" object between the main c++ code and the API... i'm experimenting with unique_ptrs right now, but i'm not sure it's the right way to do that.

i'll open a ticket for that issue too...

aoloe avatar Sep 28 '17 12:09 aoloe

Explanation: the PYBIND11_MODULE(scripterapi) macro creates (among other things) module initialization function called PyInit_scripterapi. When python loads (dlopen's) a file called scripterapi.*.so (when you call import scripterapi, that is what happens - python finds that file and dlopen's it, if it is an executable; if it is python script, it is executed, if it is directory, its init.py is executed atd), it will execute the sectup function called PyInit_scripterapi. So the PYBIND11_MODULE(document) was never executed, thus the class was not registered and converter was not found.

eudoxos avatar Sep 28 '17 12:09 eudoxos

PS before calling py::eval_file, grab the interpreter lock via py::scoped_interpreter guard{}; otherwise you might crash in multi-threaded code, as far as I know.

eudoxos avatar Sep 28 '17 12:09 eudoxos

and, of course, now i'm lost in the next issue: how to share the "document" object between the main c++ code and the API... i'm experimenting with unique_ptrs right now, but i'm not sure it's the right way to do that.

Forget about unique_ptr, it does what it says: unique ownership. Just pass raw pointer around. If you assure object lifetime by the script logic, no issue there at all.

eudoxos avatar Sep 28 '17 12:09 eudoxos

à propos the guard: https://github.com/aoloe/cpp-pybind11-playground/blob/master/scripter-api/src/scripter.h#L21

i thought it would be a good idea to treat it as a script resources and get it managed through the scripter lifetime... no idea yet, if it's a good idea or not...

aoloe avatar Sep 28 '17 12:09 aoloe

I think to pass the document instance, the best would be to declare a read-only property scripterapi.doc (py::scope().attr("doc")=py::def_readonly("doc",&docGetter) - this would be in boost::python, not sure about pybind11, how to declare scope-level attribute) and the getter would return pointer to the current document. It would be nice to work with also other documents (scribus can have multiple documents open separately), though that would be an extra.

eudoxos avatar Sep 28 '17 13:09 eudoxos

i'll try to get a version with raw pointers working this evening CET (i don't have the "code" with me...).

i think it will be a good base for discussing further improvements.

aoloe avatar Sep 28 '17 14:09 aoloe

this ticket was bout the correct way of defining the python module in a modular way.

the issue has been solved with 0be66b10f3f209c .

i suggest that the discussion about a better management of the memory / data structure can be moved to #4 and this issue can be closed.

aoloe avatar Sep 28 '17 18:09 aoloe