fix topological sort

Author: dimitri-yatsenko

datajoint/dependencies.py

@@ -4,58 +4,70 @@
 from collections import defaultdict
 from .errors import DataJointError
 
+def extract_master(part_table):
+    """
+    given a part table name, return master part. None if not a part table 
+    """
+    match = re.match(r"(?P<master>`\w+`.`#?\w+)__\w+`", part_table)
+    return match['master'] + '`' if match else None
+
+
 
 def topo_sort(graph):
     """
     topological sort of a dependency graph that keeps part tables together with their masters
     :return: list of table names in topological order
     """
+
     graph = nx.DiGraph(graph)  # make a copy
 
     # collapse alias nodes
     alias_nodes = [node for node in graph if node.isdigit()]
     for node in alias_nodes:
-        direct_edge = (
-            next(x for x in graph.in_edges(node))[0],
-            next(x for x in graph.out_edges(node))[1],
-        )
-        graph.add_edge(*direct_edge)
+        try:
+            direct_edge = (
+                next(x for x in graph.in_edges(node))[0],
+                next(x for x in graph.out_edges(node))[1],
+            )
+        except StopIteration:
+            pass  # a disconnected alias node
+        else:
+            graph.add_edge(*direct_edge)
     graph.remove_nodes_from(alias_nodes)
 
     # Add parts' dependencies to their masters' dependencies
     # to ensure correct topological ordering of the masters.
-    part_pattern = re.compile(r"(?P<master>`\w+`.`#?\w+)__\w+`")
     for part in graph:
-        # print part tables and their master
-        match = part_pattern.match(part)
-        if match:
-            master = match["master"] + "`"
+        # find the part's master
+        master = extract_master(part)
+        if master:
             for edge in graph.in_edges(part):
-                if edge[0] != master:
-                    graph.add_edge(edge[0], master)
+                parent = edge[0]
+                if parent != master and extract_master(parent) != master:
+                    graph.add_edge(parent, master)
 
-    sorted_nodes = list(nx.algorithms.topological_sort(graph))
+    sorted_nodes = list(nx.topological_sort(graph))
 
     # bring parts up to their masters
-    pos = len(sorted_nodes)
-    while pos > 0:
-        pos -= 1
+    pos = len(sorted_nodes) - 1
+    placed = set()
+    while pos > 1:
         part = sorted_nodes[pos]
-        match = part_pattern.match(part)
-        if match:
-            master = match["master"] + "`"
-            print(part, master)
+        master = extract_master(part)
+        if not master or part in placed:
+            pos -= 1
+        else:
+            placed.add(part)
             try:
                 j = sorted_nodes.index(master)
             except ValueError:
                 # master not found
-                continue
-            if pos > j + 1:
-                print(pos, j)
-                # move the part to its master
-                del sorted_nodes[pos]
-                sorted_nodes.insert(j + 1, part)
-                pos += 1
+                pass
+            else:                
+                if pos > j + 1:
+                    # move the part to its master
+                    del sorted_nodes[pos]
+                    sorted_nodes.insert(j + 1, part)
 
     return sorted_nodes
 
@@ -202,16 +214,14 @@ def descendants(self, full_table_name):
         :return: all dependent tables sorted in topological order.  Self is included.
         """
         self.load(force=False)
-        nodes = self.subgraph(
-            nx.algorithms.dag.descendants(self, full_table_name)
-        ).copy()
-        return [full_table_name] + nodes.topo_sort()
+        nodes =  self.subgraph(nx.descendants(self, full_table_name))
+        return  [full_table_name] + nodes.topo_sort()
 
     def ancestors(self, full_table_name):
         """
         :param full_table_name:  In form `schema`.`table_name`
         :return: all dependent tables sorted in topological order.  Self is included.
         """
         self.load(force=False)
-        nodes = self.subgraph(nx.algorithms.dag.ancestors(self, full_table_name)).copy()
+        nodes = self.subgraph(nx.ancestors(self, full_table_name))
         return reversed(nodes.topo_sort() + [full_table_name])

datajoint/diagram.py

@@ -6,7 +6,7 @@
 import inspect
 from .table import Table
 from .dependencies import topo_sort
-from .user_tables import Manual, Imported, Computed, Lookup, Part
+from .user_tables import Manual, Imported, Computed, Lookup, Part, _get_tier, _AliasNode
 from .errors import DataJointError
 from .table import lookup_class_name
 
@@ -27,30 +27,6 @@
 
 
 logger = logging.getLogger(__name__.split(".")[0])
-user_table_classes = (Manual, Lookup, Computed, Imported, Part)
-
-
-class _AliasNode:
-    """
-    special class to indicate aliased foreign keys
-    """
-
-    pass
-
-
-def _get_tier(table_name):
-    """given the table name, return"""
-    if not table_name.startswith("`"):
-        return _AliasNode
-    else:
-        try:
-            return next(
-                tier
-                for tier in user_table_classes
-                if re.fullmatch(tier.tier_regexp, table_name.split("`")[-2])
-            )
-        except StopIteration:
-            return None
 
 
 if not diagram_active:

datajoint/schemas.py

@@ -5,14 +5,13 @@
 import collections
 import itertools
 from .connection import conn
-from .diagram import Diagram, _get_tier
 from .settings import config
 from .errors import DataJointError, AccessError
 from .jobs import JobTable
 from .external import ExternalMapping
 from .heading import Heading
 from .utils import user_choice, to_camel_case
-from .user_tables import Part, Computed, Imported, Manual, Lookup
+from .user_tables import Part, Computed, Imported, Manual, Lookup, _get_tier
 from .table import lookup_class_name, Log, FreeTable
 import types
 
@@ -451,10 +450,8 @@ def replace(s):
                 ).replace("\n", "\n    " + indent),
             )
 
-        diagram = Diagram(self)
-        body = "\n\n".join(
-            make_class_definition(table) for table in diagram.topo_sort()
-        )
+        tables = self.connection.dependencies.topo_sort()
+        body = "\n\n".join(make_class_definition(table) for table in tables)
         python_code = "\n\n".join(
             (
                 '"""This module was auto-generated by datajoint from an existing schema"""',

datajoint/user_tables.py

@@ -2,6 +2,7 @@
 Hosts the table tiers, user tables should be derived from.
 """
 
+import re
 from .table import Table
 from .autopopulate import AutoPopulate
 from .utils import from_camel_case, ClassProperty
@@ -242,3 +243,29 @@ def drop(self, force=False):
     def alter(self, prompt=True, context=None):
         # without context, use declaration context which maps master keyword to master table
         super().alter(prompt=prompt, context=context or self.declaration_context)
+
+
+user_table_classes = (Manual, Lookup, Computed, Imported, Part)
+
+
+class _AliasNode:
+    """
+    special class to indicate aliased foreign keys
+    """
+
+    pass
+
+
+def _get_tier(table_name):
+    """given the table name, return"""
+    if not table_name.startswith("`"):
+        return _AliasNode
+    else:
+        try:
+            return next(
+                tier
+                for tier in user_table_classes
+                if re.fullmatch(tier.tier_regexp, table_name.split("`")[-2])
+            )
+        except StopIteration:
+            return None