Created attachment 328237 [details] [review]
Gtk: allow sequence treemodel overrides

Add get_ and set_ override for sequences of GtkTreeModel indices. This
allows an arbitray list if indeces to be retrieved in one go from a
GtkTreeModel.


First go at this, seems to work fine in some limited testing. Maybe 
needs a testcase as well?

Created attachment 328304 [details] [review]
overrides: allow treemodel sequence shorthands

Add get_ and set_ overrides for sequences of GtkTreeModel indices. This
allows an arbitray list of indeces to be retrieved or written in one go
from or to a GtkTreeModel row:

model[0][0, 1] = [True, "Hello"]
[foo, bar] = model[0][2, 7]

The new patch adds tests.

One thing I'm not 100% sure about is that a 1 element sequence is actually handled by the single element isIsinstance(key, int). This part could be removed in favor of the sequence handling as it is just a special case imo. See also the test line in the patch in comment #2: 1597.

Related to that is the fact that the single element list handling allows negative values, I guess as a service to the user to allow for the pythonic 'start from the end of the sequence' (row in this case). At first I had this covered for sequences as well, but then I removed it as it is completely undocumented and might be confusing. But it would be trivial to add back.

Review of attachment 328304 [details] [review]:

::: gi/overrides/Gtk.py
@@ +1080,3 @@
                 alist.append(self.model.get_value(self.iter, i))
             return alist
+        elif isinstance(key, tuple):

This could be simplified to

return [self[k] for k in key]

with the bonus that can handle a mix of indices and slices, like [1,2,3:-1]

@@ +1091,2 @@
         else:
+            raise TypeError("indices must be integers, slice or list, not %s" % type(key).__name__)

passing lists doesn't work so I would use "tuple" here to avoid confusion

@@ +1109,3 @@
             for i, v in enumerate(indexList):
                 self.model.set_value(self.iter, v, value[i])
+        elif isinstance(key, tuple):

Similar to above, can be reduced to:

if len(key) != len(value):
    raise ValueError(
        "attempt to assign sequence of size %d to sequence of size %d"
        % (len(value), len(key)))
for k, v in zip(key, value):
    self[k] = v

@@ +1122,2 @@
         else:
+            raise TypeError("indices must be an integer, slice or list, not %s" % type(key).__name__)

see above

::: tests/test_overrides_gtk.py
@@ +1630,3 @@
+
+        def set_row4():
+            model[0][0, 1, 1] = (0, "zero", "one")

While not very useful, I don't think this should be considered invalid usage.

Created attachment 329135 [details] [review]
overrides: allow treemodel sequence shorthands

Add get_ and set_ overrides for sequences of GtkTreeModel indices. This
allows an arbitray list of indeces to be retrieved or written in one go
from or to a GtkTreeModel row:

model[0][0, 1] = [True, "Hello"]
[foo, bar] = model[0][2, 7]

(In reply to Christoph Reiter (lazka) from comment #4)
Thanks for the review.

Your solution is so much cleaner, shorter and nicer, so use that instead.

> 
> ::: tests/test_overrides_gtk.py
> @@ +1630,3 @@
> +
> +        def set_row4():
> +            model[0][0, 1, 1] = (0, "zero", "one")
> 
> While not very useful, I don't think this should be considered invalid usage.

I dropped it from the patch, I guess this is harder to check for with possibly nested lists of indices.

I was not certain about this either way, but it just seems to me it might prove the source of hard to track down bugs if we allow something that makes little sense in practice and is so position dependant.

Review of attachment 329135 [details] [review]:

lgtm

This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.