Created attachment 22203 [details] [review]
the patch

This is related to bug 80915.

In order to feel fully comfortable with something like this, I would
prefer if derived GObjects acted a bit more like Python objects, so
that we don't surprise people.

What I am referring to is the requirement to call
self.__gobject_init__() on classes that have been registered with
gobject.type_register().  I have some ideas on how to solve this, but
it would require changes to all the GObject constructors in
pygtk+friends (it is probably doable though).

In essence, make things like gtk.HBox.__init__ actually call
g_object_init() with the GType of the instance being created.  This
way, if you subclass gtk.HBox in the way a Python programmer would
expect to, you would end up with instances of the correct class. 
Without a change like this, the metaclass will probably create
problems/questions rather than solving them.


It is worth noting that a patch like this would only deprecate
gobject.type_register() -- gobject.signal_new() is still necessary to
create some signals.  Consider the "parent_set" signal on GtkWidget,
for instance.  It couldn't be defined til after the GType for
GtkWidget had been registered.

It is also worth noting that if types are registered automatically,
some code that previously functioned may now fail.  Consider this:
    import gobject

    class C(gobject.GObject):
        pass
    class C(gobject.GObject):
        pass

The second class definition will fail to register, due to the type
name conflict.  We would need some form of conflict resolution to
handle this case.

  I agree with you that gobject.signal_new() is still required.  I had
already discussed with Johan about this.

  I also completely agree that the g_object_init() hack needs to be
resolved.  I hate that too...

  However, I disagree that the metaclass creates any new problems if
the g_object_init() issues is not resolved.  But, fine, you're trying
to put pressure on resolving g_object_init() too, I understand ;-) 
But resolving g_object_init() should be in a different bug report.  It
is better to take one step at a time, imho.

  But, by any chance, does your enivisoned solution involve calling
g_object_newv(), passing arguments as properties, in the python
constructor, and passing the correct GType as well?  If so, then
that's one more point we agree in ;)  And I believe it's very much
worth doing, even if it means changing all constructors.

Yeah, the plan was that eg. _wrap_gtk_hbox_new() would be changed to
look something like this:

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;
}

From what I hear from the C++ developers, gtkmm essentially does this
in all their constructors, so it should be possible to reimplement all
the constructors.  This is really a separate problem to the metaclass
one though.

Anyway, the main place where this is likely to cause problems is where
two GObject subclasses with the same name get defined (like I
mentioned earlier).  Some of the places I can imagine this occuring are:
 1) define a class within a function, then call the function
    multiple times.
 2) calling reload() on a module that defines some GObject subclasses.

*** This bug has been marked as a duplicate of 129843 ***

This is not a duplicate. It is a slightly different issue.  We just deviated
from the topic a bit in the discussion, but the metaclass issue is completely
orthogonal to the __init__ problem.

Over to patch author. Gustavo, reassign to nobody if this falls off your plate,
thanks.

I won't commit unless I get some positive feedback on this.
Personally, I think this is very useful patch, because calling
gobject.type_register(MyClass) seems a bit ugly and un-pythonic to me.  And
unlike the __init__ problem (bug 129843), this is a simple fix.

Created attachment 26768 [details] [review]
Allow registering multiple classes with the same name in same module

This patch solves the following problem pointed out by James:
    class C(gobject.GObject):
	pass
    class C(gobject.GObject):
	pass

It adds -v<n> to the type name.  Example:
class MyObject(gobject.GObject): pass
gobject.type_register(MyObject)
print MyObject.__gtype__

class MyObject(gobject.GObject): pass
gobject.type_register(MyObject)
print MyObject.__gtype__

Output is:
<GType __main__+MyObject (135076320)>
<GType __main__+MyObject-v2 (134521112)>

With this problem solved, I reapply the metaclass patch for inclusion in pygtk.
What say you?

One thing worth noting is that if we include metaclass + gtype "uniquification"
patch, we should make gobject.type_register() a no-op and give deprecation
warning.  Otherwise, this legacy code registers the same type twice, once
because of the metaclass, and another time by explicit call to
gobject.type_register:
    class MyObject(gobject.GObject): pass
    gobject.type_register(MyObject)

This last patch is useful by itself, for example when you reload() a module, as
James pointed out, so I committed it to cvs head.

Created attachment 28879 [details] [review]
Updated metaclass patch

Here's an updated patch.  Unfortunately, no matter what I do, python crashes in
an apparently unrelated function with some programs.  While
examples/gobject/signal.py works just fine, pygtk-demo crashes.  I've given up
for now.  This patch shouldn't be included in pygtk 2.4, but I'll try again
when python 2.4 comes out.

Created attachment 45652 [details] [review]
new patch, now really works solidly

Created attachment 45653 [details] [review]
improved metaclass patch

Changes:
  - A bit of refactoring;
  - gobject.type_register now gives a deprecation warning and only registers
the class if it wasn't already registered (doesn't contain a __gtype__
attribute).

Created attachment 45654 [details] [review]
minor correction: forgot to check return value of PyErr_Warn()

It is time to commit this patch.  Any objections?

Created attachment 46771 [details] [review]
cleaned up patch