Skip to content
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

support for nested classes #2032

Closed
pavlis opened this issue Dec 16, 2019 · 6 comments
Closed

support for nested classes #2032

pavlis opened this issue Dec 16, 2019 · 6 comments

Comments

@pavlis
Copy link

pavlis commented Dec 16, 2019

Issue description

I have read through the pybind11 documentation at least a dozen times, looked through all the examples, and did every permutation I could think of searching with google but cannot crack the problem of nested classes in pybind11. There are hints from some examples that this might be possible, but with no documentation on the topic and no easily found examples methinks this totally standard part of the C++ language may not be supported? The actual project I'm working on that needs this is much more complex, but the example below captures the idea I'm trying to implement.

Reproducible example code

#include
#include
#include
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/stl_bind.h>
using namespace std;
namespace py=pybind11;
/* This is a simple error logger class*/
class logger
{
public:
logger(){}; //probably unnecessary
void log_error(std::string mess)
{
messages.push_back(mess);
};
list get_log()
{
return messages;
};
private:
list messages;
};
/* This is a simple data object that includes an error logger as a

  • nested class - type example of "has a" for a nested class */
    class data_object
    {
    public:
    data_object(vector d) : v(d){};
    vector get_data(){return v;};
    logger elog;
    private:
    vector v;
    };

/* Now the pybind11 code /
PYBIND11_MODULE(test_nested,m)
{
/
Probably not essential for this test, but would be needed*/
py::bind_vector<std::vector>(m,"Vector");
py::class_(m,"logger","Simple error logger class")
.def(py::init<>())
.def("log_error",&logger::log_error)
.def("get_log",&logger::get_log)
;
py::class_<data_object>(m,"data_object","Trivial data object for test")
.def(py::init<vector>())
.def("get_data",&data_object::get_data)
/* Forms to try for logger*/
//simple reference to symbol
//.def("elog",&data_object::elog::log_error)
//variant
//.def("elog",&data_object::logger::log_error)
//lambda function modeled after pickle example
//.def("log_error",[](const std::string mess)
//{elog.log_error(mess);}) // variation 1
//{this->elog.log_error(mess);}) //variation 2
;
}

Explanations

The issue is that data_object has a nested class of logger. The idea here is to allow processing of data objects in a massively parallel system and allow errors to be posted with the data to allow after the fact sorting out problem results. This is a classic example of a nested class where data_object "has a" logger.

When I compile the above (with all attempts to wrap logger in data_object commented out, the wrappers for logger and data_object work. Comment out any of my guesses on how to do this with pybind11, however, and I get errors. Working on an ubuntu system with gcc. Here are the results:

Variant 1: .def("elog",&data_object::elog::log_error)
test_nested.cpp: In function ‘void pybind11_init_test_nested(pybind11::module&)’:
test_nested.cpp:52:29: error: ‘data_object::elog’ is not a class, namespace, or enumeration
.def("elog",&data_object::elog::log_error)

  • note I knew this was unlikely to work -

Variant 2: .def("elog",&data_object::logger::log_error)
test_nested.cpp: In function ‘void pybind11_init_test_nested(pybind11::module&)’:
test_nested.cpp:54:29: error: ‘data_object::logger’ has not been declared
.def("elog",&data_object::logger::log_error)
^~~~~~
Variant 3: lambda function variation 1:
.def("log_error",[](const std::string mess)
{elog.log_error(mess);})

test_nested.cpp: In lambda function:
test_nested.cpp:57:5: error: ‘elog’ was not declared in this scope
{elog.log_error(mess);})
^~~~
test_nested.cpp:57:5: note: suggested alternative: ‘log’
{elog.log_error(mess);})
^~~~
log
Variant 4: lambda function variation 2
.def("log_error",[](const std::string mess)
{this->elog.log_error(mess);})

/home/pavlis/src/mspass/cxx/python/mspass_wrapper.cpp: In lambda function:
/home/pavlis/src/mspass/cxx/python/mspass_wrapper.cpp:480:9: error: ‘this’ was not captured for this lambda function
this->elog.set_algorithm(alg);
^~~~
python/CMakeFiles/mspasspy.dir/build.make:62: recipe for target 'python/CMakeFiles/mspasspy.dir/mspass_wrapper.cpp.o' failed

Perhaps this has gotten too long, but wanted to demonstrate I had tried every variation I could think of. Summary is that no matter what the solution here is there is a big hole in the documentation. The documentation needs to say clearly nested classes aren't supported or add a short description of how to wrap a nested class.

@pavlis
Copy link
Author

pavlis commented Dec 22, 2019

The silence on this issue has been deafening with 6 days and no response at all! I have had to conclude at this point that nested classes are, at best, difficult to handle. Additional searching in the past few days has only confirmed that. I won't close this issue, but I quickly created a workaround that works for this case. Because our project is not some megacorporate endeavor it isn't hard for us to just change the api. The issue is actually mostly an artistic/aesthetic issue anyway. That is, l the problem noted is an example of a classic application taught for a nested class: the nested class satisfies the "has a" rule versus "is a" rule for inheritance. Thus, my solution was simple - I just made the formerly nested class become a parent of the data objects. The data objects just inherit the logger in the usual way, and the wrappers become standard. I now have a working version and it only took about 1 hour of coding and an hour or testing to verify it worked. Making it work is more important than aesthetics in most cases and this proved to be no exception.

@pavlis
Copy link
Author

pavlis commented Dec 23, 2019

The colleague I am working on with this project, Yinzhi Wang, found the right incantation to make this actually work with nested classes. i.e. it seem pybind11 does indeed support nested classes fairly cleanly, but it is just a horrible hole in the documentation. If the wrapper for the example is written as follows this works as expected:
py::class_<data_object>(m,"data_object","Trivial data object for test")
.def(py::init())
.def("get_data",&data_object::get_data)
.def_readwrite("elog",&data_object::elog,"Nested class for error logger")

Seeing the solution this now makes some sense. Since in python all data are objects elog as a class is treated no differently than stock types of integer, string, and real(float).

The unambiguous conclusion now is that this is a big hole in the documentation - a completely undocumented feature. If I knew how the documentation was handled I would write a section on this topic. I won't close this issue in hopes someone will give me instructions on how to do that - I'd be happy to contribute to this project in that way.

@bstaletic
Copy link
Collaborator

The part of the docs you didn't read is called "Instances and static fields", on the second page of the docs, called "Object-oriented code".

@YannickJadoul
Copy link
Collaborator

The silence on this issue has been deafening with 6 days and no response at all!

Welcome to the world of open source, where you paid exactly nothing to use the code and get support. Formatting your code and question layout (use ``` for code blocks!) and making sure it is correct #include <what?> would already help making this more inviting to reply to. Meanwhile, the issue templates asks/suggest that if it's just a question, you could already pass by on Gitter.

@xaedes
Copy link
Contributor

xaedes commented Oct 13, 2020

I came here looking for this feature (nested classes). I saw static fields in doc, but it didn't come to mind that this feature can be used for mapping nested classes.
But I guess this is issue is some kind of documentation...

@xaedes
Copy link
Contributor

xaedes commented Oct 13, 2020

I was actually looking how to bind nested type declerations like class Foo { struct Bar { }; };.
In case anyone wonders how to do it, I found it in the docs under section "Enumerations and internal types".
In hindsight it is obvious, instead of giving the module as first parameter to py::class_ you give it the parent class python binding:

class Foo { struct Bar { }; };
auto foo = py::class_<Foo>(m, "Foo");
auto bar = py::class_<Foo::Bar>(foo, "Bar");

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

4 participants