Fix Find Database Loop tool for faster operation and better display

Fixes #10299

gramps/plugins/tool/findloop.glade

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<!-- Generated with glade 3.16.1 -->
+<!-- Generated with glade 3.18.3 -->
 <interface>
   <requires lib="gtk+" version="3.10"/>
   <object class="GtkDialog" id="findloop">
     <property name="can_focus">False</property>
-    <property name="default_width">450</property>
+    <property name="default_width">650</property>
     <property name="default_height">400</property>
     <property name="type_hint">dialog</property>
     <signal name="delete-event" handler="on_delete_event" swapped="no"/>
@@ -18,23 +18,6 @@
             <property name="visible">True</property>
             <property name="can_focus">False</property>
             <property name="layout_style">end</property>
-            <child>
-              <object class="GtkButton" id="close">
-                <property name="label">gtk-close</property>
-                <property name="use_action_appearance">False</property>
-                <property name="visible">True</property>
-                <property name="can_focus">True</property>
-                <property name="can_default">True</property>
-                <property name="receives_default">False</property>
-                <property name="use_stock">True</property>
-                <signal name="clicked" handler="destroy_passed_object" swapped="yes"/>
-              </object>
-              <packing>
-                <property name="expand">False</property>
-                <property name="fill">False</property>
-                <property name="position">0</property>
-              </packing>
-            </child>
             <child>
               <object class="GtkButton" id="helpbutton1">
                 <property name="label">gtk-help</property>
@@ -53,7 +36,21 @@
               </packing>
             </child>
             <child>
-              <placeholder/>
+              <object class="GtkButton" id="close">
+                <property name="label">gtk-close</property>
+                <property name="use_action_appearance">False</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="can_default">True</property>
+                <property name="receives_default">False</property>
+                <property name="use_stock">True</property>
+                <signal name="clicked" handler="destroy_passed_object" swapped="yes"/>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="fill">False</property>
+                <property name="position">0</property>
+              </packing>
             </child>
           </object>
           <packing>
@@ -92,6 +89,7 @@
                   <object class="GtkTreeView" id="treeview">
                     <property name="visible">True</property>
                     <property name="can_focus">True</property>
+                    <property name="enable_search">False</property>
                     <child internal-child="selection">
                       <object class="GtkTreeSelection" id="treeview-selection1"/>
                     </child>
@@ -111,9 +109,6 @@
             <property name="position">1</property>
           </packing>
         </child>
-        <child>
-          <placeholder/>
-        </child>
       </object>
     </child>
     <action-widgets>

gramps/plugins/tool/findloop.py

@@ -18,6 +18,7 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
+from _collections import OrderedDict
 
 "Find possible loop in a people descendance"
 
@@ -43,9 +44,10 @@ from gramps.gui.utils import ProgressMeter
 from gramps.gui.display import display_help
 from gramps.gui.glade import Glade
 from gramps.gen.display.name import displayer as _nd
+from gramps.gen.proxy import CacheProxyDb
 from gramps.gen.const import GRAMPS_LOCALE as glocale
 _ = glocale.translation.sgettext
-ngettext = glocale.translation.ngettext # else "nearby" comments are ignored
+ngettext = glocale.translation.ngettext  # else "nearby" comments are ignored
 
 #-------------------------------------------------------------------------
 #
@@ -55,6 +57,7 @@ ngettext = glocale.translation.ngettext # else "nearby" comments are ignored
 WIKI_HELP_PAGE = '%s_-_Tools' % URL_MANUAL_PAGE
 WIKI_HELP_SEC = _('manual|Find_database_loop')
 
+
 #------------------------------------------------------------------------
 #
 # FindLoop class
@@ -71,6 +74,7 @@ class FindLoop(ManagedWindow):
         ManagedWindow.__init__(self, uistate, [], self.__class__)
         self.dbstate = dbstate
         self.uistate = uistate
+        #self.db = CacheProxyDb(dbstate.db)
         self.db = dbstate.db
 
         top_dialog = Glade()
@@ -96,17 +100,22 @@ class FindLoop(ManagedWindow):
             GObject.TYPE_STRING,    # 1==father
             GObject.TYPE_STRING,    # 2==son id
             GObject.TYPE_STRING,    # 3==son
-            GObject.TYPE_STRING)    # 4==family gid
+            GObject.TYPE_STRING,    # 4==family gid
+            GObject.TYPE_STRING)    # 5==loop number
+        self.model.set_sort_column_id(
+            Gtk.TREE_SORTABLE_UNSORTED_SORT_COLUMN_ID, 0)
 
         self.treeview = top_dialog.get_object("treeview")
         self.treeview.set_model(self.model)
+        col0 = Gtk.TreeViewColumn('',
+                                  Gtk.CellRendererText(), text=5)
         col1 = Gtk.TreeViewColumn(_('Gramps ID'),
                                   Gtk.CellRendererText(), text=0)
-        col2 = Gtk.TreeViewColumn(_('Ancestor'),
+        col2 = Gtk.TreeViewColumn(_('Parent'),
                                   Gtk.CellRendererText(), text=1)
         col3 = Gtk.TreeViewColumn(_('Gramps ID'),
                                   Gtk.CellRendererText(), text=2)
-        col4 = Gtk.TreeViewColumn(_('Descendant'),
+        col4 = Gtk.TreeViewColumn(_('Child'),
                                   Gtk.CellRendererText(), text=3)
         col5 = Gtk.TreeViewColumn(_('Family ID'),
                                   Gtk.CellRendererText(), text=4)
@@ -120,11 +129,7 @@ class FindLoop(ManagedWindow):
         col3.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
         col4.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
         col5.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
-        col1.set_sort_column_id(0)
+        self.treeview.append_column(col0)
-        col2.set_sort_column_id(1)
-        col3.set_sort_column_id(2)
-        col4.set_sort_column_id(3)
-        col5.set_sort_column_id(4)
         self.treeview.append_column(col1)
         self.treeview.append_column(col2)
         self.treeview.append_column(col3)
@@ -135,36 +140,50 @@ class FindLoop(ManagedWindow):
 
         self.curr_fam = None
         people = self.db.get_person_handles()
-        count = 0
+        self.total = len(people)  # total number of people to process.
+        self.count = 0  # current number of people completely processed
+        self.loop = 0  # Number of loops found for GUI
+
+        pset = OrderedDict()
+        # pset is the handle list of persons from the current start of
+        # exploration path to the current limit.  The use of OrderedDict
+        # allows us to use it as a LIFO during recursion, as well as makes for
+        # quick lookup.  If we find a loop, pset provides a nice way to get
+        # the loop path.
+        self.done = set()
+        # self.done is the handle set of people that have been fully explored
+        # and do NOT have loops in the decendent tree.  We use this to avoid
+        # repeating work when we encounter one of these during the search.
         for person_handle in people:
             person = self.db.get_person_from_handle(person_handle)
-            count += 1
             self.current = person
             self.parent = None
-            self.descendants(person_handle, set())
+            self.descendants(person_handle, pset)
-            self.progress.set_header("%d/%d" % (count, len(people)))
-            self.progress.step()
 
         # close the progress bar
         self.progress.close()
 
         self.show()
 
-    def descendants(self, person_handle, new_list):
+    def descendants(self, person_handle, pset):
         """
         Find the descendants of a given person.
+        Returns False if a loop for the person is NOT found, True if loop found
+        We use the return value to ensure a person is not put on done list if
+        part of a loop
         """
-        person = self.db.get_person_from_handle(person_handle)
+        if person_handle in self.done:
-        pset = set()
+            return False  # We have already verified no loops for this one
-        for item in new_list:
+        if person_handle in pset:
-            pset.add(item)
+            # We found one loop.
-        if person.handle in pset:
+            # person_handle is child, self.parent, self.curr_fam valid
-            # We found one loop
+            # see if it has already been put into display
-            father_id = self.current.get_gramps_id()
+            person = self.db.get_person_from_handle(person_handle)
-            father = _nd.display(self.current)
+            pers_id = person.get_gramps_id()
-            son_id = self.parent.get_gramps_id()
+            pers_name = _nd.display(person)
-            son = _nd.display(self.parent)
+            parent_id = self.parent.get_gramps_id()
-            value = (father_id, father, son_id, son, self.curr_fam)
+            parent_name = _nd.display(self.parent)
+            value = (parent_id, parent_name, pers_id, pers_name, self.curr_fam)
             found = False
             for pth in range(len(self.model)):
                 path = Gtk.TreePath(pth)
@@ -175,21 +194,66 @@ class FindLoop(ManagedWindow):
                         self.model.get_value(treeiter, 3),
                         self.model.get_value(treeiter, 4))
                 if find == value:
-                    found = True
+                    found = True  # This loop is in display model
+                    break
             if not found:
-                self.model.append(value)
+                # Need to put loop in display model.
-            return
+                self.loop += 1
-        pset.add(person.handle)
+                # place first node
+                self.model.append(value + (str(self.loop),))
+                state = 0
+                # Now search for loop beginning.
+                for hndl in pset.keys():
+                    if hndl != person_handle and state == 0:
+                        continue  # beginning not found
+                    if state == 0:
+                        state = 1  # found beginning, get first item to display
+                        continue
+                    # we have a good handle, now put item on display list
+                    self.parent = person
+                    person = self.db.get_person_from_handle(hndl)
+                    # Get the family that is both parent/person
+                    for fam_h in person.get_parent_family_handle_list():
+                        if fam_h in self.parent.get_family_handle_list():
+                            break
+                    family = self.db.get_family_from_handle(fam_h)
+                    fam_id = family.get_gramps_id()
+                    pers_id = person.get_gramps_id()
+                    pers_name = _nd.display(person)
+                    parent_id = self.parent.get_gramps_id()
+                    parent_name = _nd.display(self.parent)
+                    value = (parent_id, parent_name, pers_id, pers_name,
+                             fam_id, str(self.loop))
+                    self.model.append(value)
+            return True
+        # We are not part of loop (yet) so search descendents
+        person = self.db.get_person_from_handle(person_handle)
+        # put in the pset path list for recursive calls to find
+        pset[person_handle] = None
+        loop = False
         for family_handle in person.get_family_handle_list():
             family = self.db.get_family_from_handle(family_handle)
-            self.curr_fam = family.get_gramps_id()
             if not family:
                 # can happen with LivingProxyDb(PrivateProxyDb(db))
                 continue
             for child_ref in family.get_child_ref_list():
                 child_handle = child_ref.ref
+                self.curr_fam = family.get_gramps_id()
                 self.parent = person
-                self.descendants(child_handle, pset)
+                # if any descendants are part of loop, so is search person
+                loop |= self.descendants(child_handle, pset)
+        # we have completed search, we can pop the person off pset list
+        person_handle, dummy = pset.popitem(last=True)
+
+        if not loop:
+            # person was not in loop, so add to done list and update progress
+            self.done.add(person_handle)
+            self.count += 1
+            self.progress.set_header("%d/%d" % (self.count, self.total))
+            self.progress.step()
+            return False
+        # person was in loop...
+        return True
 
     def rowactivated_cb(self, treeview, path, column):
         """