Created attachment 175406 [details] [review]
patch

This patch starts implementing at least __length__ and __contains__

I am not sure how to implement the iterator.

You need to return an iter object from __iter__ which implements next().  Unfortunately the child API isn't expressive enough to implement __get_item__ (well you could do it but it would construct a list each time).  You might want to patch Gtk+ to add a get_nth_child API but it isn't needed.  Do something like this:

class ContainerIter(object):
    def __init__(self, children):
        self._children = children
        self._len = len(self._children)
        self._pos = 0

    def next(self):
        if (self._pos == self._len):
            return None
        
        result = self._children[self._pos]
        self._pos+=1

class Container(Gtk.Container):
    def __iter__(self):
        return ContainerIter(self.get_children())

Actually reading the pep http://www.python.org/dev/peps/pep-0234/  it looks like __getitem__ needs to be implemented for the iter.  That is easy - return self._children[i]

A related but slightly different issue would be to map GSList/GList to a transparent iterator type so that say:

   for widget in container.get_children():
       ...

Would not create the entire list, next() would return (widget)list->data and update list = list->next. If that's done, then it would be trivial implementing an iterator for the above. __iter__ = get_children would probably be enough.

Yes, it's a lot more work to make GSList/GList work as a transparent and generic iterator, especially to make something like this work:

   >>> children = container.get_children()
   >>> type(children)
   <PyGIIteratorType type=GList, container=GtkContainer, item=GtkWidget>
   >>> children
   [<GtkWidget a>, <GtkWidget b>]

__getitem__() would actually need to traverse the whole list on demand, there must be previous proof of concept for that somewhere.

Created attachment 175806 [details] [review]
Implementation of the __iter__ method

Next patch in series, should be applied after Paolo's one

Either we don't understand the scope of the problem, or it's rather trivial.

The attached patch allows to use a container in 'for' expressions, like this:


In [1]: from gi.repository import Gtk, GLib, GObject, Gdk
In [2]: import gi.overrides as overrides
In [3]: box = Gtk.VBox()
In [4]: label = Gtk.Label()
In [5]: label2 = Gtk.Label()
In [6]: label3 = Gtk.Label()
In [7]: box.add(label)
In [8]: box.add(label2)
In [9]: for w in box:
   ...:     print w
   ...: 
<Label object at 0x84cd48c (GtkLabel at 0x85eb000)>
<Label object at 0x84bf98c (GtkLabel at 0x85eb080)>

Review of attachment 175806 [details] [review]:

::: tests/test_overrides.py
@@ +60,2 @@
     def test_container(self):
+        box = Gtk.VBox() # Gtk.Box is an abstract class

Box is not an abstract class anymore in Gtk3, it is a concrete class with an "orientation" property to make it a vbox or an hbox

Created attachment 175807 [details] [review]
Cumulative patch

This is a cumulative version of both patches, the one provided by Paolo and the next, created by my son and me

Actually it may well be that the solution is trivial :)
Though I'll let the pygobject maintainers decide about the correctness.

However please add a unit test to check the iteration.

Review of attachment 175807 [details] [review]:

::: gi/overrides/Gtk.py
@@ +50,1 @@
 class Container(Gtk.Container, Widget):

You need to implement __nonzero__ (Python 2.x) and __bool__ (Python 3.x), otherwise the behavior of "if container:" will depend on if there are children in it or not, which is completely wrong.

@@ +59,3 @@
+        return iter(self.get_children())
+
+    def __getitem__(self, item):

I think that implementing __getitem__ is going a bit too far, it should not be included in IMHO. 
However if it's decided that it should be included, then __setitem__ and __delitem__ should be implemented as well for consistency.

::: tests/test_overrides.py
@@ +59,3 @@
+
+    def test_container(self):
+        box = Gtk.VBox() # Gtk.Box is an abstract class

Need to test __nonzero__ here as well, eg:

   self.assertTrue(box)

(In reply to comment #10)
> @@ +59,3 @@
> +        return iter(self.get_children())
> +
> +    def __getitem__(self, item):
> 
> I think that implementing __getitem__ is going a bit too far, it should not be
> included in IMHO.

I agree with Johan. Beside also the old pygtk did not support indexing and we want to stay as close as possible to that api

Well, according to the 2.x docs __nonzero__() is optional and if not defined, then __len__() is used instead (http://docs.python.org/reference/datamodel.html#object.__nonzero__)

Here's a simple test using python-2.x (and PyGTK-2.x, we don't have gtk-3.x yet):

In [1]: from gi.repository import Gtk, GLib, GObject, Gdk
In [2]: import gi.overrides as overrides
In [3]: box = Gtk.VBox()
In [4]: if box:
   ...:     print "Yes"
   ...: else:
   ...:     print "No!"
   ...: 
No!
In [5]: label = Gtk.Label()
In [6]: box.add(label)
In [7]: if box:
   ...:     print "Yes"
   ...: else:
   ...:     print "No!"
   ...: 
Yes

The similar behaviour is for python-3.x: http://docs.python.org/py3k/reference/datamodel.html#object.__bool__

So I'm not sure if these methods actually should be implemented

Created attachment 175820 [details] [review]
Implementation of the __iter__ method

Here's the improved version

Created attachment 175825 [details] [review]
Implementation of the __iter__ method

Use Gtk.Box() instead of Gtk.VBox() in tests

Review of attachment 175825 [details] [review]:

What Johan meant (unless I misunderstood) is that __nonzero__ needs to be implemented exactly because it should not depend on len(), it should always return True independently from the fact that the container has children.

Also note that python 3 has __bool__ instead of __nonzero___ so make sure to override one or the other depending on "if sys.version_info >= (3, 0):"

::: tests/test_overrides.py
@@ +70,3 @@
+        self.assertTrue(bool(box))
+        l = [x for x in box]
+        self.assertEqual(len(l),2)

I'd assert that l equals [label, label2]

Created attachment 175833 [details] [review]
Implementation of the __iter__ method

Test results of iterator as suggested

Created attachment 175834 [details] [review]
Explicit cast to bool

Explicit cast to bool to make sure that a container is always "True"

patches looks good to me

Review of attachment 175834 [details] [review]:

The title is completely wrong, there no such a thing as 'cast' in python.

::: gi/overrides/Gtk.py
@@ +64,3 @@
+    else:
+        def __nonzero__(self):
+            return True

Again, Look at the Gtk.TreeModel in the same file for an example on how to do the same in 3 lines.

::: tests/test_overrides.py
@@ +60,3 @@
     def test_container(self):
         box = Gtk.Box()
+        self.assertTrue(bool(box))

It doesn't hurt but there should be no need for the bool() part

Created attachment 176015 [details] [review]
iterable

Updated patch with the comments fixed.

Review of attachment 176015 [details] [review]:

Looks great.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.