GNOME Bugzilla – Bug 601181
Add interfaces support to pygi
Last modified: 2009-11-28 15:49:04 UTC
This just adds the ability to discover and use interface types, implementing interfaces will come once https://bugzilla.gnome.org/show_bug.cgi?id=557383 is solved.
Created attachment 147221 [details] [review] Register interfaces
Created attachment 147222 [details] [review] Treat GI_INFO_TYPE_INTERFACE same as GI_INFO_TYPE_OBJECT
Created attachment 147223 [details] [review] Start tests about interfaces
Review of attachment 147221 [details] [review]: ::: gi/module.py @@ +126,3 @@ metaclass = GObjectMeta bases = (gobject.GInterface,) + info.register() Do that in GObjectMeta.__init__. ::: gi/pygi-info.c @@ +1741,3 @@ +{ +initialize_interface (GTypeInterface *iface, PyTypeObject *pytype) +static void Everywhere else, I used g_type rather than gtype, or simply type when there is no ambiguity between a Py or a G type. @@ +1742,3 @@ +{ +initialize_interface (GTypeInterface *iface, PyTypeObject *pytype) +static void info instead of info_struct is fine; @@ +1752,3 @@ + + Py_INCREF(Py_None); + return Py_None; Use Py_RETURN_NONE. @@ +1758,3 @@ { "get_constants", (PyCFunction)_wrap_g_interface_info_get_constants, METH_NOARGS }, { "get_methods", (PyCFunction)_wrap_g_interface_info_get_methods, METH_NOARGS }, + { "register", (PyCFunction)_wrap_g_interface_info_register, METH_NOARGS }, It seems cleaner to me to do something like _wrap_pyg_set_object_has_new_constructor for pyg_register_interface_info. There is no method named g_interface_info_register, so this implementation is misleading, IMHO.
Review of attachment 147222 [details] [review]: Looks OK.
Review of attachment 147223 [details] [review]: I don't get the point of having many methods in the interface. When one implement an interface in Python, does one expect the virtual functions to be bound to the Python methods somehow?
(In reply to comment #6) > Review of attachment 147223 [details] [review]: > > I don't get the point of having many methods in the interface. > > When one implement an interface in Python, does one expect the virtual > functions to be bound to the Python methods somehow? Yes, and we need quite a bit of code to marshall the arguments from C to Python, so we'll need some more test cases. But those will better come in a ticket about implementing vfuncs in python.
Created attachment 148269 [details] [review] Register interfaces
Created attachment 148271 [details] [review] Register interfaces
Review of attachment 148271 [details] [review]: Almost perfect :-) ::: gi/gimodule.c @@ +120,3 @@ + info->interface_finalize = NULL; + info->interface_data = NULL; + No need to set them again to NULL (g_new0 does it already). @@ +121,3 @@ + info->interface_data = NULL; + + g_type = pyg_type_from_object(py_g_type); Perhaps we want to check that the g_type is a G_TYPE_INTERFACE?
Created attachment 148277 [details] [review] Register interfaces
Review of attachment 148277 [details] [review]: Excellent. Thanks!
The following fix has been pushed: 1dc62a9 Register interfaces
Created attachment 148280 [details] [review] Register interfaces
And what about the tests?
(In reply to comment #15) > And what about the tests? Coming with the vfuncs stuff, but can be split here if you prefer to.
Created attachment 148283 [details] [review] A few tests about interfaces
Ping, this is a very easy patch to review.
Review of attachment 148283 [details] [review]: ::: tests/test_gi.py @@ +1432,3 @@ +class TestInterfaces(unittest.TestCase): + +# Interface I'd prefer if you could split this tests: one for the interface wrapper, one for the implementation in Python.
Created attachment 148642 [details] [review] A few tests about interfaces
Review of attachment 148642 [details] [review]: Good.
The following fix has been pushed: 96f6c63 A few tests about interfaces
Created attachment 148651 [details] [review] A few tests about interfaces