Created attachment 180966 [details] [review]
Skip interfaces when checking for conflicts in the MRO

Comment on attachment 180966 [details] [review]
Skip interfaces when checking for conflicts in the MRO

Tests works, demos work, looks good...commit to both master and python-2-28 branch

Review of attachment 180966 [details] [review]:

::: gi/types.py
@@ +242,3 @@
+    seqs = [[C]] + map(mro, C.__bases__) + [list(C.__bases__)]
+    res = []
+    """Compute the class precedence list (mro) according to C3

spaces around =

@@ +243,3 @@
+    res = []
+    i=0
+

True

@@ +244,3 @@
+    i=0
+    while 1:
+

Kill this, it's inefficient, can be done using else: after the other for.

@@ +250,3 @@
+        for seq in nonemptyseqs: # find merge candidates among seq heads
+            cand = seq[0]
+    """

Use a generator for efficiency + any()

::: tests/test_gi.py
@@ +1542,3 @@
+        class TestInterfaceImpl2(GIMarshallingTests.Interface,
+                                 TestInterfaceImpl):
+

Would be good to add a few more tests here, with deeper inheritance chain

Created attachment 181710 [details] [review]
Skip interfaces when checking for conflicts in the MRO

(In reply to comment #3)
> Review of attachment 180966 [details] [review]:

Thanks for the review, initially I tried not to change too much the code from the place I found it but in retrospect it would have been better to rewrite it to suit our code standards, so that's what I have been done now.

About the generator, I'm not sure where do you think it would be a good idea.

And about the tests, have added one more case, but it's difficult to further complicate the hierarchy because of GType limitations or real MRO conflicts.

Comment on attachment 181710 [details] [review]
Skip interfaces when checking for conflicts in the MRO

>From 4896ef3eefdf839fa36084875046dded61228d9a Mon Sep 17 00:00:00 2001
>From: Tomeu Vizoso <tomeu.vizoso@collabora.co.uk>
>Date: Wed, 16 Feb 2011 09:44:12 +0100
>Subject: [PATCH] Skip interfaces when checking for conflicts in the MRO
>
>https://bugzilla.gnome.org/show_bug.cgi?id=642437
>---
> gi/types.py             |   39 +++++++++++++++++++++++++++++++++++++++
> gobject/gobjectmodule.c |    9 +++++++--
> tests/test_gi.py        |   16 ++++++++++++++++
> 3 files changed, 62 insertions(+), 2 deletions(-)
>
>diff --git a/gi/types.py b/gi/types.py
>index c2a8b35..8c324ee 100644
>--- a/gi/types.py
>+++ b/gi/types.py
>@@ -229,6 +229,45 @@ class GObjectMeta(gobject.GObjectMeta, MetaClassHelper):
>             elif isinstance(cls.__info__, InterfaceInfo):
>                 register_interface_info(cls.__info__.get_g_type())
> 
>+    def mro(cls):
>+        return mro(cls)
>+
>+
>+def mro(C):
>+    """Compute the class precedence list (mro) according to C3
>+
>+    Based on http://www.python.org/download/releases/2.3/mro/
>+    Modified to consider that interfaces don't create the diamond problem
>+    """
>+    bases = []
>+    bases_of_subclasses = [[C]]
>+
Add a TODO: profile to see if a generator would be more efficent
>+    if C.__bases__:
>+        bases_of_subclasses += map(mro, C.__bases__) + [list(C.__bases__)]
>+
>+    while bases_of_subclasses:
>+        for subclass_bases in bases_of_subclasses:
>+            candidate = subclass_bases[0]
>+            not_head = [s for s in bases_of_subclasses if candidate in s[1:]]
>+            if not_head and gobject.GInterface not in candidate.__bases__:
>+                candidate = None # conflict, reject candidate
>+            else:
>+                break
>+


Looks good.  Please commit for RC release.

Pushed to master as http://git.gnome.org/browse/pygobject/commit/?id=525f21d1365c24488b768955362085bf82512dee

Will backport to 2.28