After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 701843 - Add support for Gtk Composite Templates
Add support for Gtk Composite Templates
Status: RESOLVED OBSOLETE
Product: pygobject
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 764774 (view as bug list)
Depends on: 685218 712197
Blocks: 727919
 
 
Reported: 2013-06-08 12:24 UTC by Ignacio Casal Quinteiro (nacho)
Modified: 2018-04-12 18:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tests: Move TestSignals from test_everything into test_signal (8.59 KB, patch)
2014-05-28 04:37 UTC, Simon Feltman
committed Details | Review
refactor: Move builder connection utilities outside of Builder class (5.90 KB, patch)
2014-05-28 04:38 UTC, Simon Feltman
committed Details | Review
Add optional flags argument to Object.connect (8.40 KB, patch)
2014-05-28 04:53 UTC, Simon Feltman
none Details | Review
Add GObject class and instance init hooks (8.78 KB, patch)
2014-05-28 04:53 UTC, Simon Feltman
none Details | Review
Add support for GTK+ Composite Templates (20.69 KB, patch)
2014-05-28 04:53 UTC, Simon Feltman
none Details | Review
Add support for GTK+ Composite Templates (20.91 KB, patch)
2014-05-28 05:14 UTC, Simon Feltman
none Details | Review
Add support for GTK+ Composite Templates (20.91 KB, patch)
2014-05-28 06:00 UTC, Simon Feltman
none Details | Review
Add support for GTK+ Composite Templates (20.88 KB, patch)
2014-05-29 02:21 UTC, Simon Feltman
none Details | Review
Add support for GTK+ Composite Templates (22.63 KB, patch)
2016-08-24 13:33 UTC, Georges Basile Stavracas Neto
none Details | Review
Add support for GTK+ Composite Templates (25.78 KB, patch)
2016-08-25 23:25 UTC, Georges Basile Stavracas Neto
none Details | Review
Add support for GTK+ Composite Templates (25.69 KB, patch)
2016-08-26 05:13 UTC, Georges Basile Stavracas Neto
needs-work Details | Review
pygobject: keep __gsignals__ dict after consuming it (1.21 KB, patch)
2016-08-26 05:13 UTC, Georges Basile Stavracas Neto
none Details | Review

Description Ignacio Casal Quinteiro (nacho) 2013-06-08 12:24:34 UTC
As you may know gtk+ 3.9.x provides now a new feature called templates, would be cool if pygobject as vala does already could support this new feature.
Comment 1 Simon Feltman 2013-06-17 15:12:21 UTC
There is a comment on Tristan's blog by Jon Nordby describing some ideas for this:

http://blogs.gnome.org/tvb/2013/05/29/composite-templates-lands-in-vala/comment-page-1/#comment-3774

Essentially he advocates using decorators which I agree would be the nice way to go. I think something like the following could work for the class:

@Gtk.Template(ui="/org/foo/my/mywidget.ui")
class MyWidget(Gtk.Box):
    pass

The decorator would need to do some modification to the class object, so it would take more than tweaking the __init__ method and most likely needs to mess with the metaclass.

In terms of callbacks and "child" accessor bindings there are many approaches that can be taken from fully automatic to something more explicit:

@Gtk.Template(ui="/org/foo/my/mywidget.ui")
class MyWidget(Gtk.Box):
    # "entry" is a property bound to a child element in the ui template
    entry = Gtk.Template.Child()

    @Gtk.Template.Callback
    def button_clicked (self, button):
        pass

An automatic approach:

@Gtk.Template
class MyWidget(Gtk.Box):
    def button_clicked (self, button):
        pass

In the above, the ui file could be located by looking in the same folder as the py file and signals auto connected by default. I think we should minimally 
support auto connecting signals at least with a keyword arg (as you can with Gtk.Builder):

@Gtk.Template(ui="/org/foo/my/mywidget.ui", auto_connect=True)
class MyWidget(Gtk.Box):
    def button_clicked (self, button):
        pass
Comment 2 Simon Feltman 2013-09-20 23:16:52 UTC
Update with a bit of research: I think we will need to add support of a new meta method on GObject metaclasses (__gclassinit__) which would be called during GObject class initialization. There are already hooks in PyGObject for class init but only as part of the "C" API which is potentially dead code or at least untested. We could re-use this for calling Python hooks:
https://git.gnome.org/browse/pygobject/tree/gi/_gobject/gobjectmodule.c?id=3.9.92#n868

By checking the PyTypeObjects dictionary for the custom method __gclassinit__ and call it passing both pyclass and gclass. We also need to complete bug 685218 prior to this being able to work with the class object.

From the Python side of things, Gtk.Widget would add a custom metaclass:

# gi/overrides/Gtk.py
class WidgetMeta(GObject.ObjectMeta):
    def __gclassinit__(cls, gtkwidgetclass...):
        gtkwidgetclass.set_template(cls.template_resource)

class Widget(Gtk.Widget):
    __metaclass__ = WidgetMeta


API user perspective then sets either custom class variable or uses a decorator to set things up as described in comment #1.

class Foo(Gtk.Grid):
    template_resource = 'foo'

or:

@Gtk.Template(ui="/org/foo/my/mywidget.ui")
class Foo(Gtk.Grid):
    pass
Comment 3 Simon Feltman 2014-01-13 20:29:36 UTC
Adding bug 712197 as a requirement because adding new hooks to pygobject will destroy those patches. We really need to get bug 712197 reviewed!
Comment 4 Simon Feltman 2014-05-28 04:37:24 UTC
The following fix has been pushed:
f127fab tests: Move TestSignals from test_everything into test_signal
Comment 5 Simon Feltman 2014-05-28 04:37:28 UTC
Created attachment 277352 [details] [review]
tests: Move TestSignals from test_everything into test_signal

Move these tests into a more meaningful location.
Comment 6 Simon Feltman 2014-05-28 04:38:46 UTC
The following fix has been pushed:
ba8380d refactor: Move builder connection utilities outside of Builder class
Comment 7 Simon Feltman 2014-05-28 04:38:48 UTC
Created attachment 277353 [details] [review]
refactor: Move builder connection utilities outside of Builder class

Move _extract_handler_and_args and _builder_connect_callback into module
scope for re-use by GTK+ Composite Templates.
Comment 8 Simon Feltman 2014-05-28 04:53:11 UTC
Created attachment 277354 [details] [review]
Add optional flags argument to Object.connect

Override GObject.Object.connect() to take an optional "flags" keyword
argument which accepts GObject.ConnectFlags enum values. This is used to
support user data with swapping support (ConnectFlags.SWAPPED) and is
needed for supporting GTK+ Composite Templates.
Comment 9 Simon Feltman 2014-05-28 04:53:15 UTC
Created attachment 277355 [details] [review]
Add GObject class and instance init hooks

Add _gclass_init_ and _ginstance_init_ hooks which can be implemented on
GObject sub-classes. These map to GObject GClassInitFunc and GInstanceInitFunc
respectively and will be used for supporting GTK+ Composite Templates in
Python.
Comment 10 Simon Feltman 2014-05-28 04:53:18 UTC
Created attachment 277356 [details] [review]
Add support for GTK+ Composite Templates

Add Gtk.Template, Gtk.Template.Child, and Gtk.Template.Callback classes
which can be used for decorating Gtk.Widget sub-classes which bind them
to builder XML templates. See examples and tests for details.
Add tests and demos showing off how to use this.
Comment 11 Simon Feltman 2014-05-28 05:14:23 UTC
Created attachment 277357 [details] [review]
Add support for GTK+ Composite Templates

Grammer fixes.
Comment 12 Simon Feltman 2014-05-28 06:00:21 UTC
Created attachment 277358 [details] [review]
Add support for GTK+ Composite Templates

Renamed _builder_connect_callback2 to _template_connect_callback.
Comment 13 Paolo Borelli 2014-05-28 12:31:27 UTC
It is really great to see this finally happening!

I have read through the patches and could not spot anything particularly bad or suspicious, but then again metaclasses etc are above my python level

I also read the template docs and I saw that it searches for resource files, so that clears one of my doubts

The only thing that looks odd in the API is button = Gtk.Template.Child()
It looks strange to be using a constructor and end up with another kind of object. It is also fairly verbose.

Maybe the decorator can add a get_child() method to the template to retrieve them? Or insert a "children" dictionary so that one can simply do button = children.button ? Not sure if there are technical limitations that prevent this...
Comment 14 Simon Feltman 2014-05-28 22:56:22 UTC
(In reply to comment #13)
> I also read the template docs and I saw that it searches for resource files, so
> that clears one of my doubts

I'll make sure to add a gresource test for this as well.

> The only thing that looks odd in the API is button = Gtk.Template.Child()
> It looks strange to be using a constructor and end up with another kind of
> object. It is also fairly verbose.

The Child class is implementing the Python descriptor protocol. So it's really very similar to how a GObject Property is defined:

  class Spam(GObject.Object):
      eggs = GObject.Property(int)

So, perhaps naming it differently would be clearer?

  @Gtk.Template(ui='spam.ui')
  class Spam(Gtk.Container):
      eggs = Gtk.Template.ChildProperty()

Other ideas: ChildGetter, ChildAccessor, Gtk.TemplateProperty...

> Maybe the decorator can add a get_child() method to the template to retrieve
> them? Or insert a "children" dictionary so that one can simply do button =
> children.button ? Not sure if there are technical limitations that prevent
> this...

I think this could work, what I dislike about injecting new methods is there is the potential for masking future methods coming from base classes. I actually ran into a related problem with gtk_buildable_get_name() and gtk_widget_get_name(), the latter masks the former so I needed to use Gtk.Buildable.get_name(widget) to retrieve the builder ID which is different from the widget name.

We could avoid this by overriding the existing gtk_widget_get_template_child() [1] to work with the Python template implementation. It would be safe to clobber this since we are not using gtk_widget_class_bind_template_child() to actually bind child widgets (we don't use memory locations for binding template children in the GObject class init).

Another thing to consider is get_child() needs to do a recursive search on each access, whereas the descriptor protocol caches the result after the first call. So perhaps there is a middle ground which is still idiomatic Python:

  @Gtk.Template(ui='spam.ui')
  class Spam(Gtk.Container):
      def __init__(self, **kwargs):
          super().__init__(**kwargs)
          self.eggs = self.get_template_child(Gtk.Button, 'eggs')

This would avoid both potential expense and conflict mentioned above while still looking like fairly normal Python to a reader. How does this sound?

[1] https://developer.gnome.org/gtk3/unstable/GtkWidget.html#gtk-widget-get-template-child
Comment 15 Simon Feltman 2014-05-29 02:21:47 UTC
Created attachment 277426 [details] [review]
Add support for GTK+ Composite Templates

Updated patch removes the Gtk.Template.Child and makes get_template_child work.
The get_template_child should be available and working as it is part of the
defined C API. We can add new descriptors or convenience API on top of this if
it is deemed to verbose.
Comment 16 Paolo Borelli 2014-05-29 08:34:06 UTC
> The Child class is implementing the Python descriptor protocol. So it's really
> very similar to how a GObject Property is defined:
> 

I do not know... for a property things are very clear because you can get/set it etc. For a template child it seems to me that the fact that it is implementing the descriptor protocol is detail we do not want to expose in the api. After all once you have it, a button is just a button, you can ref it and remove it from the template and it would still be a button without being a child :)

> So perhaps there is a middle ground which is still idiomatic Python:
> 
>   @Gtk.Template(ui='spam.ui')
>   class Spam(Gtk.Container):
>       def __init__(self, **kwargs):
>           super().__init__(**kwargs)
>           self.eggs = self.get_template_child(Gtk.Button, 'eggs')
> 
> This would avoid both potential expense and conflict mentioned above while
> still looking like fairly normal Python to a reader. How does this sound?
> 

Yes, I like this idea.


(In reply to comment #15)
> We can add new descriptors or convenience API on top of this if
> it is deemed to verbose.

What about the idea of injecting a "children" dictionary? This would not have the problem of conflicting with methods and would be similar to "props"
Comment 17 Simon Feltman 2014-05-30 02:20:21 UTC
(In reply to comment #16)
> > The Child class is implementing the Python descriptor protocol. So it's really
> > very similar to how a GObject Property is defined:
> > 
> 
> I do not know... for a property things are very clear because you can get/set
> it etc. For a template child it seems to me that the fact that it is
> implementing the descriptor protocol is detail we do not want to expose in the
> api. After all once you have it, a button is just a button, you can ref it and
> remove it from the template and it would still be a button without being a
> child :)

I don't see a whole lot of difference between the following properties (either conceptually or in terms of Python implementation detail):

@Gtk.Template(ui='spam.ui')
class Spam(Gtk.Container):

    ham = GObject.Property(type=Gtk.Button, name='ham')

    eggs = Gtk.Template.ChildBinding(type=Gtk.Button, name='eggs')


The 'eggs' property is automatically bound to a child widget somewhere in the hierarchy whereas 'ham' is initially empty. I don't think either of these examples expose the fact that they are Python descriptors any more than the other, it is just what they are. In both cases we are constructing a descriptor object as part of an outer class definition. When accessing this object on an instance of the outer class, we get a different thing back than what was constructed.

> > So perhaps there is a middle ground which is still idiomatic Python:
> > 
> >   @Gtk.Template(ui='spam.ui')
> >   class Spam(Gtk.Container):
> >       def __init__(self, **kwargs):
> >           super().__init__(**kwargs)
> >           self.eggs = self.get_template_child(Gtk.Button, 'eggs')
> > 
> > This would avoid both potential expense and conflict mentioned above while
> > still looking like fairly normal Python to a reader. How does this sound?
> > 
> 
> Yes, I like this idea.

Unfortunately it turns out I was a bit confused by how this API works. The default implementation of get_template_child() requires a child binding to be setup at class initialization time with gtk_widget_class_bind_template_child_full(). I initially thought the binding method was not going to work at all in Python but it turns out it does by using a struct_offset of 0.
So there isn't a need to inject the hacky recursive search override in Python. However, making proper use of this requires a class init time binding like the descriptors mentioned above (or by some other means).

I've logged bug 730974 requesting that the get_template_child() API is loosened up so it automatically creates the binding if it doesn't exist.

> (In reply to comment #15)
> > We can add new descriptors or convenience API on top of this if
> > it is deemed to verbose.
> 
> What about the idea of injecting a "children" dictionary? This would not have
> the problem of conflicting with methods and would be similar to "props"

Since GtkContainer has get_children() and "children" here has different semantics than what we need, I think it would be a little confusing. What we are trying to achieve is the semantics of gtk_builder_get_object() which retrieves any arbitrarily nested child in the widget hierarchy whereas get_children() only returns direct children of the container.
Comment 18 Simon Feltman 2014-08-12 05:56:25 UTC
Comment on attachment 277354 [details] [review]
Add optional flags argument to Object.connect

This patch is no longer needed as connect_data() was added which accepts connect_flags: https://git.gnome.org/browse/pygobject/commit/?id=581acc4
Comment 19 Simon Feltman 2015-03-01 06:47:37 UTC
Review of attachment 277426 [details] [review]:

I think I'd like to take another shot at using the descriptor protocol as some of the original patches used.
Comment 20 Dustin Spicuzza 2015-05-18 00:18:07 UTC
More food for thought here.. today I did a separate implementation of this without realizing it was already in progress. It's very similar to what you've done (since we're both drawing inspiration from the comment on the blog post), but I think your implementation is a bit more complete and deals with edge cases better than mine does. However, if someone wants to use it now, mine works for what I've tested it on so far and (more importantly) doesn't depend on modifications to PyGI.

https://github.com/virtuald/pygi-composite-templates

I also prefer declaring the attributes in the class and then at init_template() time I replace all the attributes. I think it's less verbose than making the user call get_template_child(). I tried the descriptor protocol and throwing if it were accessed before init_template were called, but couldn't figure out how to find the attributes by only using inspect.

I found that I had to pass MyWidget to get_template_child(), and *not* the type that I was trying to retrieve. I'm not really sure why that is, it probably has something to do with the ui file I was using... but that does make it so the user only needs to set the name of the child, and not the type.

One oddity I ran into was that if you declare a child that isn't in the template, it pretty breaks the entire template and no signals can be attached nor can you retrieve template children. This seems strange to me, given that everything else in GTK just issues a GTK-Critical or whatever when this type of thing occurs.
Comment 21 Dustin Spicuzza 2015-05-21 04:55:30 UTC
I've been playing with this some more, and came to a couple of key realizations -- namely that I can get the template instance from the GtkBuilder object passed to the connect function, which means the user no longer needs to set the user-data in glade for signals. 

I've updated my implementation to use GtkTemplate.Callback and .Child like the patch here.

I released 0.2.0 of my implementation on github/pypi. From my experimentation with adding it to Exaile, this is mostly complete -- except the user still needs to call init_template().
Comment 22 Igor Gnatenko 2015-07-09 16:32:19 UTC
Any news here?
Comment 23 Dustin Spicuzza 2015-10-24 17:40:31 UTC
I've been using my external implementation for a few months now, and haven't ran into any significant problems. It'd be great to get this functionality into mainline though.

One thing that isn't clear to me from the docs is whether you can have a nested templates.. eg, Widget A inherits from Widget B, and both A and B have associated UI files with them. It seems like this isn't supported, so I'm pushing some changes to my implementation that explicitly disallow that. However, if someone can point out how that might be possible, I can change it.
Comment 24 Tristan Van Berkom 2015-10-24 18:26:18 UTC
(In reply to Dustin Spicuzza from comment #23)
> I've been using my external implementation for a few months now, and haven't
> ran into any significant problems. It'd be great to get this functionality
> into mainline though.
> 
> One thing that isn't clear to me from the docs is whether you can have a
> nested templates.. eg, Widget A inherits from Widget B, and both A and B
> have associated UI files with them. It seems like this isn't supported, so
> I'm pushing some changes to my implementation that explicitly disallow that.
> However, if someone can point out how that might be possible, I can change
> it.

This is certainly supported, and you can find examples of this pattern in GTK+ itself, GtkDialog for instance exposes the content and action area widgets publicly; this allows derived dialogs to add content to those children.
Comment 25 Dustin Spicuzza 2015-10-24 18:46:54 UTC
Ah, I see that now. Unforunately, it doesn't look like I can setup the object correctly in python without additional work under the hood in pygobject.

I'm actually having issues just allowing general subclassing (never mind nested templates), because calling init_template passes in the current instance, and when init_template does "template = GTK_WIDGET_GET_CLASS (widget)->priv->template;" the class isn't the correct type (it instead is the derived class), and so template is NULL. That might be what simon was trying to accomplish with the _ginstance_init_ functions in the PyGObject patch.
Comment 26 Dustin Spicuzza 2015-10-24 20:18:46 UTC
Unless I'm missing some deep magic somewhere, it appears to be impossible to inherit from a python object that uses a GtkTemplate, because init_template must be called from within the instance initializer, which you can't do from python. 

Simon's PyGObject patch that allows a _ginstance_init_ function should solve this problem, however.
Comment 27 Dustin Spicuzza 2015-10-24 20:47:55 UTC
Also, whenever someone gets around to this patch, can you please add tests to ensure that inheritance works? Both for the case of nested templates, and just for simple inheritance also.
Comment 28 Georges Basile Stavracas Neto 2016-08-24 13:33:59 UTC
Created attachment 334081 [details] [review]
Add support for GTK+ Composite Templates

I took the liberty to update this patch, fix some errors on connect() and also improved the demo to have a FooWidget subclass.
Comment 29 Patrick Griffis (tingping) 2016-08-24 19:37:57 UTC
Review of attachment 334081 [details] [review]:

> Filename of GTK+ Builder UI file on disk or in a gresource. Disk is
> searched first in the location of the Python classes file and assumed
> to be part of a gresource if the file is not found.

Shouldn't that be reversed since one does disk IO and the other doesn't?
Comment 30 Patrick Griffis (tingping) 2016-08-24 19:42:05 UTC
(In reply to Patrick Griffis (tingping) from comment #29)
> Review of attachment 334081 [details] [review] [review]:
> 
> > Filename of GTK+ Builder UI file on disk or in a gresource. Disk is
> > searched first in the location of the Python classes file and assumed
> > to be part of a gresource if the file is not found.
> 
> Shouldn't that be reversed since one does disk IO and the other doesn't?

Also I don't even see the logic that does this?

See also: https://github.com/virtuald/pygi-composite-templates/blob/master/gi_composites.py#L250-L262
Comment 31 Georges Basile Stavracas Neto 2016-08-25 23:25:47 UTC
Created attachment 334191 [details] [review]
Add support for GTK+ Composite Templates

You're right, TingPing. Updated the patch to try and load the resources first, and fallback to file if it fails.

Also, I updated the patch to make it load one of the classes as a GResource (and works quite well!).
Comment 32 Patrick Griffis (tingping) 2016-08-26 02:19:34 UTC
Review of attachment 334191 [details] [review]:

So I started porting a real application from pygi-composite-templates to this: https://github.com/pithos/pithos/tree/wip/upstream-templates

Issues:

- Using __gtype_name__ no longer works:

  • File "/home/tingping/Projects/pithos/pithos/AboutPithosDialog.py", line 21 in <module>
    class AboutPithosDialog(Gtk.AboutDialog):
  • File "/home/tingping/jhbuild/install/lib/python3.5/site-packages/gi/overrides/Gtk.py", line 319 in __call__
    return meta_type(cls.__name__, cls.__bases__, members)
  • File "/home/tingping/jhbuild/install/lib/python3.5/site-packages/gi/types.py", line 213 in __init__
    super(GObjectMeta, cls).__init__(name, bases, dict_)
  • File "/home/tingping/jhbuild/install/lib/python3.5/site-packages/gi/types.py", line 193 in __init__
    cls._type_register(cls.__dict__)
  • File "/home/tingping/jhbuild/install/lib/python3.5/site-packages/gi/types.py", line 205 in _type_register
    _gobject.type_register(cls, namespace.get('__gtype_name__'))     RuntimeError: could not create new GType: AboutPithosDialog (subclass of GtkAboutDialog)
  • File "/home/tingping/Projects/pithos/pithos/PreferencesPithosDialog.py", line 85 in __init__
    super().__init__(*args, use_header_bar=1, **kwargs)     TypeError: super(type, obj): obj must be an instance or subtype of type
  • File "/home/tingping/Projects/pithos/pithos/application.py", line 109 in do_activate
    self.window = PithosWindow(self, self.test_mode)
  • File "/home/tingping/Projects/pithos/pithos/pithos.py", line 135 in __init__
    self.prefs_dlg.connect('login-changed', self.pandora_reconnect)
TypeError: <PreferencesPithosDialog.PreferencesPithosDialog object at 0x7efde1df69d8 (PreferencesPithosDialog at 0x1fa1f40)>: unknown signal name: login-changed

- Random nonsensical error, possibly related to the gsignal issue?

    Traceback (most recent call last):
      File "/home/tingping/Projects/pithos/pithos/PreferencesPithosDialog.py", line 149, in on_account_changed
        if not self.email_entry.get_text() or not self.password_entry.get_text():
    AttributeError: 'Entry' object has no attribute 'email_entry'

    self is a Gtk.Dialog...?


Comments:

I find the syntax for get_template_child() slightly verbose. Not a deal breaker but would be good to think about if the type information is needed?


If you want to build the Pithos project to test it yourself simply run `./autogen.sh --prefix=$JHBUILD_PREFIX && make run`.
Comment 33 Patrick Griffis (tingping) 2016-08-26 02:31:21 UTC
Bugzilla ate some of my comment as a traceback so I'll just repeat that.

- super() (with no args) fails.
- __gsignals__ don't seem to work.
Comment 34 Georges Basile Stavracas Neto 2016-08-26 05:13:35 UTC
Created attachment 334199 [details] [review]
Add support for GTK+ Composite Templates

No need to specify the child type now.
Comment 35 Georges Basile Stavracas Neto 2016-08-26 05:13:53 UTC
Created attachment 334200 [details] [review]
pygobject: keep __gsignals__ dict after consuming it

After consuming the signals, PyGObject removes it from
the object. The new works on composite templates, however,
requires that this specific dictionary stays available
after being consumed by the class_init() handler.

This patch stops class_init() from removing the __gsignals__
dict from the object.
Comment 36 Patrick Griffis (tingping) 2016-08-26 06:03:39 UTC
Ok so the only remaining things for me:

- super() still doesn't work for me (but does for feaneron?):

    TypeError: super(type, obj): obj must be an instance or subtype of type

- Would be nice for the first arg on callbacks to be `self`.
Comment 37 Tristan Van Berkom 2016-09-01 07:12:35 UTC
Glad to see there is more progress here :)

Curiously, does the nested templated subclassing work at this point ? (see comment 23 and comment 24).
Comment 38 Patrick Griffis (tingping) 2016-09-01 09:28:27 UTC
(In reply to Tristan Van Berkom from comment #37)
> Glad to see there is more progress here :)
> 
> Curiously, does the nested templated subclassing work at this point ? (see
> comment 23 and comment 24).

In a simple test, no: https://paste.gnome.org/p4q1eigvk/dsdmqo


Also just to expand upon my previous comment the issue with super()
is that Gtk.Template.__call__() returns a new type so that fails.
I'm not too familiar with 'metaclasses' to know exactly what the
correct solution is.
Comment 39 Damián Nohales 2017-03-13 20:12:42 UTC
Review of attachment 334199 [details] [review]:

::: gi/overrides/Gtk.py
@@ +160,3 @@
+
+
+        cls.set_template(gbytes)

Looks like we are not using gtk_widget_get_template_child here but a workaround to not use such a bad designed method (I mean, why gtk_widget_get_template_child requires a widget_type? that's really redundant, that information is already in the XML definition). The problem is that we are still mentioning widget_type in the docs and assuming that get_template_child accepts widget_type in demos and tests.
Comment 40 Damián Nohales 2017-03-13 20:44:29 UTC
Review of attachment 334199 [details] [review]:

::: gi/overrides/Gtk.py
@@ +160,3 @@
+
+
+        cls.set_template(gbytes)

I think I misread the purpose of widget_type here. widget_type is not the child type, but the "The GType to get a template child for" and is used, for example, to get a child for a super-class, from a sub-class.

So, I think get_template_child should have a signature like this:

def get_template_child(widget, name, widget_type=None):
    ...

Being widget_type = widget's type by default.
Comment 41 Tristan Van Berkom 2017-03-14 06:40:20 UTC
Trying to keep my eye on this...

(In reply to Damián Nohales from comment #40)
> Review of attachment 334199 [details] [review] [review]:
> 
> ::: gi/overrides/Gtk.py
> @@ +160,3 @@
> +
> +
> +        cls.set_template(gbytes)
> 
> I think I misread the purpose of widget_type here. widget_type is not the
> child type, but the "The GType to get a template child for" and is used, for
> example, to get a child for a super-class, from a sub-class.
> 
> So, I think get_template_child should have a signature like this:
> 
> def get_template_child(widget, name, widget_type=None):
>     ...
> 
> Being widget_type = widget's type by default.

So just to clarify, as I wrote in the docs for gtk_widget_get_template_child() but may have not been clear enough:

  "This function is only meant to be called for code which is private to the 
   widget_type which declared the child and is meant for language bindings 
   which cannot easily make use of the GObject structure offsets."

To clarify further... it is very bad style, and hopefully disallowed completely, for a child class to call this in order to snoop around and access a child widget that belongs to a super class.

So last year I learned a bit of python (not so much the implementation) so I could illustrate a bit how I might expect the api to be exposed to python GTK+ users.


@template('/org/foo/mywidget.ui')
@child('entry')
@callback('button_clicked')
class MyWidget(GtkDialog):

  # button_clicked()
  #
  # This callback was automatically bound to a callback
  # named 'button_clicked' inside the 'mywidget.ui' file.
  #
  # Python bindings used:
  #   gtk_widget_class_bind_template_callback_full()
  #
  # to declare that connection on the GType for MyWidget and provided
  # a C callback to propagate that call to a function on this class.
  #
  def button_clicked(self):

    # Here we can access the self.entry widget, because Python
    # bindings used:
    #
    #    gtk_widget_class_bind_template_child_full()
    #
    # To create an access point for the child widget when creating
    # the GType for MyWidget.
    #
    # Later, in MyWidget.__init__(), Python automatically made the
    # assignment:
    #
    #    self.entry = gtk_widget_get_template_child()
    #
    self.entry.set_text('Hello World')


So in an ideal world, the following class decorators:


  @template()

  Causes the specified ui file to be wrapped up in a gresource
  and used as the class's template.

  --
  This results in gtk_widget_class_set_template_from_resource()
  being called in the GObject class initializer for the widget,
  and also results in gtk_widget_init_template() to be called
  in the instance initializer.


  @child()

  Causes the specified child name to automatically appear on
  instances of the python class.

  --
  This results in gtk_widget_class_bind_template_child_full()
  to be called in the GObject class initializer, telling GTK+ to
  keep track of the child instance whenever instantiating this
  class (will happen deep inside gtk_widget_init_template().

  Also, it results in automatically binding:

    self.foo = gtk_widget_get_template_child()


  @callback()

  Binds a class method to be bound to a callback name which was
  declared in the .ui file, so the class method will be called
  whenever that signal is emitted.

  --
  Under the hood, this results in gtk_widget_class_bind_template_callback_full()
  being called in the class initializer, passing along a C callback which
  will be used to propagate that call forward to the class method.


Does this make more sense ?
Comment 42 Patrick Griffis (tingping) 2017-03-14 18:44:24 UTC
(In reply to Tristan Van Berkom from comment #41)
> @template('/org/foo/mywidget.ui')
> @child('entry')
> @callback('button_clicked')
> class MyWidget(GtkDialog):


Just from an API perspective I believe we can all agree on @template. @callback could probably be moved to the function itself and accomplish the same thing, which would be cleaner and more clear?

@child though is a bit of an ugly one though, the class could easily have a dozen children and that is ugly to define as a class decorator. It also will work very poorly with tools, self.entry isn't being clearly defined so most IDEs will trip up on that.
Comment 43 Simon Feltman 2017-03-14 23:27:19 UTC
(In reply to Patrick Griffis (tingping) from comment #42)
> Just from an API perspective I believe we can all agree on @template.
> @callback could probably be moved to the function itself and accomplish the
> same thing, which would be cleaner and more clear?

Yeah, these should be method decorators otherwise it isn't clearly coupled and won't scale as the class gets bigger.

> @child though is a bit of an ugly one though, the class could easily have a
> dozen children and that is ugly to define as a class decorator. It also will
> work very poorly with tools, self.entry isn't being clearly defined so most
> IDEs will trip up on that.

I still like the property descriptor idea for this:

    @Gtk.Template(ui='breakfast.ui')
    class Breakfast(Gtk.Container):
        spam = Gtk.Template.Child('spam')

However, I'm a bit torn if the constructor should explicitly take the name of the child or it is magically deduced from the attribute name:

    @Gtk.Template(ui='breakfast.ui')
    class Breakfast(Gtk.Container):
        spam = Gtk.Template.Child()

The former feels redundant while the latter seems too magical as we are using external description to deduce internal naming. We can error on the side of being redundant as we can always go from redundant to magic in an API change, but not the other way around.
Comment 44 Patrick Griffis (tingping) 2017-03-15 02:25:18 UTC
(In reply to Simon Feltman from comment #43)
> I still like the property descriptor idea for this:
> 
>     @Gtk.Template(ui='breakfast.ui')
>     class Breakfast(Gtk.Container):
>         spam = Gtk.Template.Child('spam')
> 
> However, I'm a bit torn if the constructor should explicitly take the name
> of the child or it is magically deduced from the attribute name:
> 
>     @Gtk.Template(ui='breakfast.ui')
>     class Breakfast(Gtk.Container):
>         spam = Gtk.Template.Child()
> 
> The former feels redundant while the latter seems too magical as we are
> using external description to deduce internal naming. We can error on the
> side of being redundant as we can always go from redundant to magic in an
> API change, but not the other way around.

I think automatic is fine (its what the decorators are effectively doing) but a way to override the name would be nice.

This is the API we are already using for a while now and works well IMO:
https://github.com/TingPing/transmission-remote-gnome/blob/master/trg/window.py#L40-L53
Comment 45 Dustin Spicuzza 2017-03-15 02:37:20 UTC
I haven't thought this all the way through yet, and I'm not sure how well this would work because of potential late binding issues... but this seems fitting. A variable injection pattern that I've been using for a year now in a different context[1] is something like this:

  class Foo:
    varname = Type

The injector scans the constructed Foo object looking for types, and then replacing them on the constructed instance with the injected variable. Taking that idea and creating an example that makes sense here:
  
  @Gtk.Template(ui='template.ui')
  class MyWidget(Gtk.Container):
    button = Gtk.Button

In this example, at MyWidget construction time, it would notice there's a 'button' variable, and try to ask the template for the variable named 'button', and would throw if the types don't match. Like I said, I haven't thought this through in this context, but it's a useful technique and might make sense here.

One nice thing about this technique is that it enables autocomplete for the variables in IDEs such as pydev.

[1]: http://robotpy.readthedocs.io/en/latest/frameworks/magicbot.html#variable-injection
Comment 46 Tristan Van Berkom 2017-03-15 06:13:35 UTC
(In reply to Patrick Griffis (tingping) from comment #42)
> (In reply to Tristan Van Berkom from comment #41)
> > @template('/org/foo/mywidget.ui')
> > @child('entry')
> > @callback('button_clicked')
> > class MyWidget(GtkDialog):
> 
> 
> Just from an API perspective I believe we can all agree on @template.
> @callback could probably be moved to the function itself and accomplish the
> same thing, which would be cleaner and more clear?
> 
> @child though is a bit of an ugly one though, the class could easily have a
> dozen children and that is ugly to define as a class decorator. It also will
> work very poorly with tools, self.entry isn't being clearly defined so most
> IDEs will trip up on that.

I agree with you and others that the API would be even more ideal if we had decorators on the class methods and instance properties themselves, for declaration of @callback methods and @child variables respectively.

I jumped to conclusions with the idea that you might not be able to achieve what is needed if you dont use class wide decorators, because each of these decorators, need to actually install things in the class initializer of the GType you create for a python derived GTK+ widget (this was a naive assumption to make as I have no idea about the implementation details of how you go from parsing python to generating GTypes).

In any case, it's better to not expose any API for users to call gtk_widget_get_template_child() in python, this API is really only intended for consumption by the binding code itself, not to users of the binding.
Comment 47 Patrick Griffis (tingping) 2017-03-15 17:24:53 UTC
(In reply to Tristan Van Berkom from comment #46)
> In any case, it's better to not expose any API for users to call
> gtk_widget_get_template_child() in python, this API is really only intended
> for consumption by the binding code itself, not to users of the binding.

Is there any real difference between:

  @Gtk.Template.Child('foo')
  class Foo:

and:

  class Foo:
    foo = Gtk.Template.Child()
Comment 48 Dustin Spicuzza 2017-03-15 18:03:54 UTC
(In reply to Patrick Griffis (tingping) from comment #47)
> (In reply to Tristan Van Berkom from comment #46)
> > In any case, it's better to not expose any API for users to call
> > gtk_widget_get_template_child() in python, this API is really only intended
> > for consumption by the binding code itself, not to users of the binding.
> 
> Is there any real difference between:
> 
>   @Gtk.Template.Child('foo')
>   class Foo:
> 
> and:
> 
>   class Foo:
>     foo = Gtk.Template.Child()

If one were to go that route, you might as well do:
  
  @Gtk.Template(ui='something', children=['child1', 'child2'])
  class Foo:
    pass

I don't like the idea of setting the children via the decorator.
Comment 49 Tristan Van Berkom 2017-03-15 18:43:10 UTC
(In reply to Patrick Griffis (tingping) from comment #47)
> (In reply to Tristan Van Berkom from comment #46)
> > In any case, it's better to not expose any API for users to call
> > gtk_widget_get_template_child() in python, this API is really only intended
> > for consumption by the binding code itself, not to users of the binding.
> 
> Is there any real difference between:
> 
>   @Gtk.Template.Child('foo')
>   class Foo:
> 
> and:
> 
>   class Foo:
>     foo = Gtk.Template.Child()

I dont think you're reading exactly what I'm saying here.

Either of the above, is not the same as gtk_widget_get_template_child(), which allows one to peek at children which are private to the declaring class. I.e. gtk_widget_get_template_child() is there so that you can implement one of the above two, but it _is not_ either of them.

Also, remember that gtk_widget_get_template_child() by itself is _not_ enough, either of the above forms require that the binding have another consequence, which is that gtk_widget_class_bind_template_child_full() _must_ be called for that child in the GType class initializer (i.e. the _class_init() ) for the Foo type above.

As I said in my last comment, it was naive of me to think all the decorators need to go on the class itself and not elsewhere because I'm not familiar with how the binding is implemented. But, the reason I thought that was logical: is because every one of these things, _must_ have a side effect on how the pygobject binding defines the class, i.e. each one has to make calls on the underlying GType class initializer, in C. I was naively assuming that once you are not at the scope of defining a class, you may no longer have the opportunity to register things in the GType class initializer.



One more thing which came to mind, is be careful about not providing any way to specify the name of the child widget in which every way you expose the API, I'm thinking specifically of subclassing composite templated widgets, where the subclass should not really have to care that it's parent has a widget named "entry", or that it may grow an "entry" in the future.

At least with python you have some basic namespacing features, so you might at least get away with:

  @template("foo.ui")
  class FooWidget(GtkBox):

    # This assigns the "entry" bound to this class from "foo.ui"
    __entry = Gtk.Template.Child()


So that later when you have:

  @template("bar.ui")
  class BarWidget(FooWidget):

    # This assigns a different "entry" bound to this class from "bar.ui"
    __entry = Gtk.Template.Child()


You can be sure that both classes may bind a child named "entry", and they wont step on eachother.
Comment 50 Daniel Boles 2017-10-09 11:41:08 UTC
*** Bug 764774 has been marked as a duplicate of this bug. ***
Comment 51 GNOME Infrastructure Team 2018-01-10 20:27:45 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/pygobject/issues/52.
Comment 52 Christoph Reiter (lazka) 2018-04-04 21:04:23 UTC
I've opened https://gitlab.gnome.org/GNOME/pygobject/merge_requests/52

Based on Dustin's implementation, except init_template() doesn't need to be called and Callback/Child take a name argument + more error checking and tests
Comment 53 Christoph Reiter (lazka) 2018-04-12 18:16:20 UTC
(In reply to Christoph Reiter (lazka) from comment #52)
> I've opened https://gitlab.gnome.org/GNOME/pygobject/merge_requests/52

It's in master now