GNOME Bugzilla – Bug 685076
Add override for Gtk.Container.child_get_property
Last modified: 2014-05-26 06:06:50 UTC
The child_get_property method is currently not introspectable due to its usage of GValue as an in/out argument. This could be fixed by creating an override which simply re-implements the method using the workaround of "child.get_property".
Created attachment 225569 [details] [review] Add an override for Gtk.Container.child_get() As part of that, also add GObject.Value.get() bindings that will be useful outside of that.
(In reply to comment #1) > Created an attachment (id=225569) [details] [review] > Add an override for Gtk.Container.child_get() > > As part of that, also add GObject.Value.get() bindings that > will be useful outside of that. This depends on the patches attached to bug 685218
from the patch: + elif gtype == constants.TYPE_OBJECT: + return self.get_object() + # TYPE_PYOBJECT + elif gtype == constants.TYPE_OBJECT: + return self.get_object() maybe a copy/paste typo?
Review of attachment 225569 [details] [review]: Looks good, a few comments. ::: gi/overrides/GObject.py @@ +81,3 @@ + raise TypeError("Unsupported GType: %s" % (gtype, )) + +__all__.append('Value') This seems to basically duplicate the code in gi/_gobject/pygtype.c:pyg_value_as_pyobject. It would be great to refactor this and remove the static bindings or somehow share the code, similar to what is noted in: https://bugzilla.gnome.org/show_bug.cgi?id=685275 Minimally a comment should go in both the static binding code and python override referencing a ticket url and description that the code is duplicated so readers and hackers can easily know about it. ::: gi/overrides/Gtk.py @@ +93,3 @@ + values.append(value.get(pspec.value_type)) + return values + value = GObject.Value() It would be nice to override child_get_property and put all the GObject.Value code in there. get_child could then use the overridden version. Passing by reference for out args is just ugly in python and I think we should abstract that whenever possible in pygobject.
(In reply to comment #3) > from the patch: > > + elif gtype == constants.TYPE_OBJECT: > + return self.get_object() > + # TYPE_PYOBJECT > + elif gtype == constants.TYPE_OBJECT: > + return self.get_object() > > maybe a copy/paste typo? Yeah, good catch. Thanks. (In reply to comment #4) > Review of attachment 225569 [details] [review]: > > Looks good, a few comments. > > ::: gi/overrides/GObject.py > @@ +81,3 @@ > + raise TypeError("Unsupported GType: %s" % (gtype, )) > + > +__all__.append('Value') > > This seems to basically duplicate the code in > gi/_gobject/pygtype.c:pyg_value_as_pyobject. It would be great to refactor this > and remove the static bindings or somehow share the code, similar to what is > noted in: https://bugzilla.gnome.org/show_bug.cgi?id=685275 > Minimally a comment should go in both the static binding code and python > override referencing a ticket url and description that the code is duplicated > so readers and hackers can easily know about it. Agree. I think that most of the static bindings should be removed in the long run. It'll help on many levels, easier to contribute, debug, understand, run under other python implementations just to mention a few. However, it's actually quite a bit of work, and sometimes there needs to be help in glib with updated annotations, api etc. Meanwhile while we do this migration, I don't think it's necessarily a bad thing to do keep two implementations of the same. Would it suffice to add a FIXME/TODO to the C static version saying that the python-only version should be preferred? > ::: gi/overrides/Gtk.py > @@ +93,3 @@ > + values.append(value.get(pspec.value_type)) > + return values > + value = GObject.Value() > > It would be nice to override child_get_property and put all the GObject.Value > code in there. get_child could then use the overridden version. Passing by > reference for out args is just ugly in python and I think we should abstract > that whenever possible in pygobject. Yeah, I guess you're right. It's just that child_get_property() were never available inside PyGTK, but in the new introspection world it makes sense to implement both and make child_get() return a list.
(In reply to comment #5) > > ::: gi/overrides/GObject.py > > @@ +81,3 @@ > > + raise TypeError("Unsupported GType: %s" % (gtype, )) > > + > > +__all__.append('Value') > > > > This seems to basically duplicate the code in > > gi/_gobject/pygtype.c:pyg_value_as_pyobject. It would be great to refactor this > > and remove the static bindings or somehow share the code, similar to what is > > noted in: https://bugzilla.gnome.org/show_bug.cgi?id=685275 > > Minimally a comment should go in both the static binding code and python > > override referencing a ticket url and description that the code is duplicated > > so readers and hackers can easily know about it. > > Agree. I think that most of the static bindings should be removed in the long > run. It'll help on many levels, easier to contribute, debug, understand, run > under other python implementations just to mention a few. However, it's > actually quite a bit of work, and sometimes there needs to be help in glib with > updated annotations, api etc. I am just now starting to grasp the basics of how all this fits together and it has been many months of hacking. I think piecemeal refactoring is the right approach if possible to accomplish what you mention. And yes it will be easier to contribute, maintain, etc... For instance, I added the "freeze_notify" context managers in the C bindings. This could have been done in literally a few lines of python but it ended up being a nice chunk of hand crafted C code. I created a meta ticket for tracking work related to removal of the static bindings. It would be great to hear more of your insight and ideas for how to approach some of this as I would like to contribute to the effort. https://bugzilla.gnome.org/show_bug.cgi?id=685373 > Meanwhile while we do this migration, I don't think it's necessarily a bad > thing to do keep two implementations of the same. Would it suffice to add a > FIXME/TODO to the C static version saying that the python-only version should > be preferred? Agreed. Just some bread crumbs will due.
*** Bug 652941 has been marked as a duplicate of this bug. ***
Review of attachment 225569 [details] [review]: A lot of code has changed since this patch was created: GObject.py overrides have already gone in with an override for GValue, I think the patch could probably re-based and become much simpler.
FYI style_get_property has the same problem
I met this problem also. Is there a plan to fix this problem in upstream? I have not found any related fix in latest upstream git repository.
(In reply to comment #10) > I met this problem also. Is there a plan to fix this problem in upstream? > I have not found any related fix in latest upstream git repository. Note that you can still get child properties, it is just a bit obtuse: #### from gi.repository import Gtk box = Gtk.Box() label = Gtk.Label() box.pack_start(label, expand=False, fill=True, padding=38) value = GObject.Value(bool) box.child_get_property(label, 'expand', value) value.get_boolean() ####
still error: Traceback (most recent call last):
+ Trace 232874
value = GObject.Value(bool)
Should I install the latest pygobject from git?
I'm not sure of the exact version this was introduced (definitely PyGObject 3.8+ though). You might also try: value = GObject.Value() value.init(bool)
Good, it works, however it's not so elegant. Expecting an evolution in future. Thank you!
Fix for child_get_property was pushed: https://git.gnome.org/browse/pygobject/commit/?id=6f5a9a37bcdec5
The following fix has been pushed: 45a5fb2 overrides: Make value argument to Widget.style_get_property optional
Created attachment 277175 [details] [review] overrides: Make value argument to Widget.style_get_property optional Override Gtk.Widget.style_get_property to optionally accept the "value" argument. If "value" is not supplied, the override will locate the child property value type and create the GValue. Additionally return the resulting GValue converted to a native Python value.
The following fix has been pushed: d0b23f0 overrides: Add Gtk.Container.child_get/set overrides
Created attachment 277176 [details] [review] overrides: Add Gtk.Container.child_get/set overrides Add overrides for child_get and child_set to Gtk.Container since these are not introspectable methods.