GNOME Bugzilla – Bug 129843
Make all constructors work through g_object_new()
Last modified: 2006-01-09 14:11:32 UTC
One of the warts in pygtk w.r.t. defining new widget types is that the standard way of chaining up to the superclass's constructor does not act as expected. The following code creates a C level instance of GtkHBox rather than MyBox: class MyBox(gtk.HBox): def __init__(self): gtk.HBox.__init__(self) gobject.type_register(MyBox) To get the desired behaviour, the gtk.HBox.__init__(self) call needs to be replaced with self.__gobject_init__() or gobject.GObject.__init__(self). To get rid of this inconsistency, we would need to modify all GObject constructors to create C level instances of the wrapper class's type. This would require using g_object_new() rather than eg. gtk_hbox_new(). This might make it impossible to auto-generate constructors for GObjects, but it is probably worth it. An example constructor can be found in the comments of bug 128765 (GObject metaclass).
*** Bug 80915 has been marked as a duplicate of this bug. ***
*** Bug 128765 has been marked as a duplicate of this bug. ***
This needs to get on the radar.
For reference: HBox.__init__ static int _wrap_gtk_hbox_new(PyGObject *self, PyObject *args, PyObject *kwargs) { static char *kwlist[] = { "homogeneous", "spacing", NULL }; GType obj_type; int homogeneous = FALSE, spacing = 0; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|ii:GtkHBox.__init__", kwlist, &homogeneous, &spacing)) return -1; obj_type = pyg_type_from_object(self); self->obj = g_object_new(obj_type, "homogeneous", homogeneous, "spacing", spacing, NULL); if (!self->obj) { PyErr_SetString(PyExc_RuntimeError, "could not create GtkHBox object"); return -1; } pygobject_register_wrapper((PyObject *)self); return 0; }
Which is different from the current one: - self->obj = (GObject *)gtk_hbox_new(homogeneous, spacing); + obj_type = pyg_type_from_object(self); + self->obj = g_object_new(obj_type, + "homogeneous", homogeneous, + "spacing", spacing, + NULL); Shouldn't be too difficult to modify the code generator.
Created attachment 29186 [details] [review] Generate constructors that use g_object_newv Basically this patch makes the code generator generate constructors that use g_object_newv, thus fixing this bug. The defs will have to be slightly changed. Take a look at the sample change in HBox constructor. Only the property names are necessary, not their types, nor default values. Currently, argument names and property names must be the same, but the infrastructure is there to support argument names different from the properties. Of course, overriden code is not affected, but I guess we can fix that code with time... Some minor cleanup required, but basically I'm happy with this patch. Feedback most welcome.
Thanks a lot for working on this. Patch looks good, some minor comments: pyg_parse_constructor_args is quite complicated, this is fine because it does quite a bit. But a more verbose documentation/doc comment would be nice. Is it possible to do a map over a GParameter*, instead of creating an extra variable (counter) and just call this function with the number of arguments? If there isn't something in glib (similar to g_list_foreach), then it's fine as it is right now We're changing the output of the code generator slightly (on stderr, progress output), not sure if it's a good ide. new-style is a bad name, property-style is better, to avoid confusion with new-style classes. I hate "if foo: bar". Which code requires Parameter/Property to be treated as a four sized tuple?
The stderr messages **writing new-style constructor** are just debugging messages, will be removed. The rest of style changes I will take care of. Still, perhaps it is a good idea to generate warning when creating 'old-style' constructors? Because they trigger this bug and should be replaced sooner or later--if not with property-style constructor, then with an override. After the code is in, a major effort of changing all constructors in the .defs awaits us... not to mention updating the manual constructors.. :-|
Created attachment 29221 [details] Example metaclass that excepts arbitrary properties via keyword arguments Example metaclass base class used to help implement gobject subclasses along with a trivial example. My suggestion is to allow any property to be set via keyword arguments to the object constuctor, so the something like the following can be used to create and init a widget: w = gtk.Alignment(xscale = 1.0, yscale = 0.0, xalign = 0.0, yalign = 0.5, visible = True, child = gtk.HBox(visible = True, spacing = 2)) The file may not work as is because it was copied out of a larger file where it does work.
Property based constructors patch committed, with some cleanup/fixes. More work required to convert all (parameter ...) to (property ...) of constructors. Now I need sleep..
Created attachment 29295 [details] defs parameters <-> properties checker I wrote a small script that scans .defs, finds all constructors, and compares all parameters agains the list of properties of the corresponding object. It then warns about mismatches between param names and property names. It seems to suggest that, in module gtk, a few constructors will require different names between properties and parameters. Also, a very small number of constructors don't have corresponding properties, not even with different name. These need to be overriden, I'm afraid.
Created attachment 29363 [details] [review] Patch to support argname != propname (already in cvs)
http://cvs.gnome.org/viewcvs/gnome-python/pygtk/gtk/gtk.defs?r1=1.183&r2=1.184&diff_format=u So now we can subclass most widgets without calling self.__gobject_init__ ;-) A couple of constructors could not be converted, due to missing properties. Also, the constructors in .override files (such as gtk.Dialog) need manual changes, but those changes shouldn't be difficult. The other modules, gdk and pango, don't need the same treatment, do they? Who would want to subclass gdk.Window or pango.Font?
http://cvs.gnome.org/viewcvs/gnome-python/pygtk/ChangeLog?r1=1.949&r2=1.950&diff_format=u http://cvs.gnome.org/viewcvs/gnome-python/pygtk/gtk/gtk.override?r1=1.273&r2=1.274&diff_format=u Please, beware of my bugs... :-P Alas... I didn't take care of all widgets, yet...
Please revert the GtkTable. It breaks in gtk_table_resize (called from gazpacho somewhere) It's actually a Gtk+ bug, since it says that rows can be 0 to MAXUINT, but it asserts in resize if it's not higher than 0. Or is gazpacho perhaps doing something weird?
The problem is different defaults for n_rows and n_columns. PyGtk used to have default=1, while gtk+ uses default=0, which is invalid. IMHO, gtk+ is right, and the current pygtk API is wrong, because it makes no sense to have n_rows and n_columns parameters optional, since there is no value that can be considered a good default. Certainly the 1x1 default is not useful because a table with n_rows=1 is like an HBox, and n_columns=1 is like a VBox. Can we break the API and make rows, cols mandatory arguments? We _are_ allowed to break the API for now, you know... ;-)
Perhaps we can throw a warning instead that not specifying any arguments is deprecated? One might otherwise argue that all classes/types in PyGTK should be able to be created without specifying any arguments. (as long as it's possible of course)
Fixed gtk.Table defaults. No warning given, though. Maybe later..
I think this is generally fixed, unless I missed one or two objects...