Merge branch 'master' into docker-img-refactor

Author: ethho

.github/workflows/development.yaml

@@ -110,7 +110,7 @@ jobs:
           flake8 --ignore=E203,E722,W503 datajoint \
                  --count --max-complexity=62 --max-line-length=127 --statistics \
                  --per-file-ignores='datajoint/diagram.py:C901'
-          black --required-version '24.2.0' --check -v datajoint tests tests_old
+          black --required-version '24.2.0' --check -v datajoint tests
   codespell:
     name: Check for spelling errors
     permissions:

CHANGELOG.md

@@ -5,7 +5,9 @@
 - Fixed - Added encapsulating double quotes to comply with [DOT language](https://graphviz.org/doc/info/lang.html) - PR [#1177](https://github.com/datajoint/datajoint-python/pull/1177)
 - Added - Datajoint python CLI ([#940](https://github.com/datajoint/datajoint-python/issues/940)) PR [#1095](https://github.com/datajoint/datajoint-python/pull/1095)
 - Added - Ability to set hidden attributes on a table - PR [#1091](https://github.com/datajoint/datajoint-python/pull/1091)
-- Added - Ability to specify a list of keys to popuate - PR [#989](https://github.com/datajoint/datajoint-python/pull/989)
+- Added - Ability to specify a list of keys to populate - PR [#989](https://github.com/datajoint/datajoint-python/pull/989)
+- Fixed - fixed topological sort [#1057](https://github.com/datajoint/datajoint-python/issues/1057)- PR [#1184](https://github.com/datajoint/datajoint-python/pull/1184)
+- Fixed - .parts() not always returning parts [#1103](https://github.com/datajoint/datajoint-python/issues/1103)- PR [#1184](https://github.com/datajoint/datajoint-python/pull/1184)
 
 ### 0.14.2 -- Aug 19, 2024
 - Added - Migrate nosetests to pytest - PR [#1142](https://github.com/datajoint/datajoint-python/pull/1142)

datajoint/dependencies.py

@@ -5,28 +5,64 @@
 from .errors import DataJointError
 
 
-def unite_master_parts(lst):
+def extract_master(part_table):
     """
-    re-order a list of table names so that part tables immediately follow their master tables without breaking
-    the topological order.
-    Without this correction, a simple topological sort may insert other descendants between master and parts.
-    The input list must be topologically sorted.
-    :example:
-    unite_master_parts(
-        ['`s`.`a`', '`s`.`a__q`', '`s`.`b`', '`s`.`c`', '`s`.`c__q`', '`s`.`b__q`', '`s`.`d`', '`s`.`a__r`']) ->
-        ['`s`.`a`', '`s`.`a__q`', '`s`.`a__r`', '`s`.`b`', '`s`.`b__q`', '`s`.`c`', '`s`.`c__q`', '`s`.`d`']
+    given a part table name, return master part. None if not a part table
     """
-    for i in range(2, len(lst)):
-        name = lst[i]
-        match = re.match(r"(?P<master>`\w+`.`#?\w+)__\w+`", name)
-        if match:  # name is a part table
-            master = match.group("master")
-            for j in range(i - 1, -1, -1):
-                if lst[j] == master + "`" or lst[j].startswith(master + "__"):
-                    # move from the ith position to the (j+1)th position
-                    lst[j + 1 : i + 1] = [name] + lst[j + 1 : i]
-                    break
-    return lst
+    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:
+        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.
+    for part in graph:
+        # find the part's master
+        if (master := extract_master(part)) in graph:
+            for edge in graph.in_edges(part):
+                parent = edge[0]
+                if master not in (parent, extract_master(parent)):
+                    # if parent is neither master nor part of master
+                    graph.add_edge(parent, master)
+    sorted_nodes = list(nx.topological_sort(graph))
+
+    # bring parts up to their masters
+    pos = len(sorted_nodes) - 1
+    placed = set()
+    while pos > 1:
+        part = sorted_nodes[pos]
+        if (master := extract_master(part)) not in graph or part in placed:
+            pos -= 1
+        else:
+            placed.add(part)
+            insert_pos = sorted_nodes.index(master) + 1
+            if pos > insert_pos:
+                # move the part to the position immediately after its master
+                del sorted_nodes[pos]
+                sorted_nodes.insert(insert_pos, part)
+
+    return sorted_nodes
 
 
 class Dependencies(nx.DiGraph):
@@ -131,6 +167,10 @@ def load(self, force=True):
             raise DataJointError("DataJoint can only work with acyclic dependencies")
         self._loaded = True
 
+    def topo_sort(self):
+        """:return: list of tables names in topological order"""
+        return topo_sort(self)
+
     def parents(self, table_name, primary=None):
         """
         :param table_name: `schema`.`table`
@@ -167,22 +207,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))
-        return unite_master_parts(
-            [full_table_name] + list(nx.algorithms.dag.topological_sort(nodes))
-        )
+        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))
-        return list(
-            reversed(
-                unite_master_parts(
-                    list(nx.algorithms.dag.topological_sort(nodes)) + [full_table_name]
-                )
-            )
-        )
+        nodes = self.subgraph(nx.ancestors(self, full_table_name))
+        return reversed(nodes.topo_sort() + [full_table_name])

datajoint/diagram.py

@@ -1,12 +1,11 @@
 import networkx as nx
-import re
 import functools
 import io
 import logging
 import inspect
 from .table import Table
-from .dependencies import unite_master_parts
-from .user_tables import Manual, Imported, Computed, Lookup, Part
+from .dependencies import topo_sort
+from .user_tables import Manual, Imported, Computed, Lookup, Part, _get_tier, _AliasNode
 from .errors import DataJointError
 from .table import lookup_class_name
 
@@ -27,29 +26,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):
-    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:
@@ -59,8 +35,7 @@ class Diagram:
         Entity relationship diagram, currently disabled due to the lack of required packages: matplotlib and pygraphviz.
 
         To enable Diagram feature, please install both matplotlib and pygraphviz. For instructions on how to install
-        these two packages, refer to http://docs.datajoint.io/setup/Install-and-connect.html#python and
-        http://tutorials.datajoint.io/setting-up/datajoint-python.html
+        these two packages, refer to https://datajoint.com/docs/core/datajoint-python/0.14/client/install/
         """
 
         def __init__(self, *args, **kwargs):
@@ -72,19 +47,22 @@ def __init__(self, *args, **kwargs):
 
     class Diagram(nx.DiGraph):
         """
-        Entity relationship diagram.
+        Schema diagram showing tables and foreign keys between in the form of a directed
+        acyclic graph (DAG).  The diagram is derived from the connection.dependencies object.
 
         Usage:
 
         >>>  diag = Diagram(source)
 
-        source can be a base table object, a base table class, a schema, or a module that has a schema.
+        source can be a table object, a table class, a schema, or a module that has a schema.
 
         >>> diag.draw()
 
         draws the diagram using pyplot
 
         diag1 + diag2  - combines the two diagrams.
+        diag1 - diag2  - difference between diagrams
+        diag1 * diag2  - intersection of diagrams
         diag + n   - expands n levels of successors
         diag - n   - expands n levels of predecessors
         Thus dj.Diagram(schema.Table)+1-1 defines the diagram of immediate ancestors and descendants of schema.Table
@@ -94,6 +72,7 @@ class Diagram(nx.DiGraph):
         """
 
         def __init__(self, source, context=None):
+
             if isinstance(source, Diagram):
                 # copy constructor
                 self.nodes_to_show = set(source.nodes_to_show)
@@ -154,7 +133,7 @@ def from_sequence(cls, sequence):
 
         def add_parts(self):
             """
-            Adds to the diagram the part tables of tables already included in the diagram
+            Adds to the diagram the part tables of all master tables already in the diagram
             :return:
             """
 
@@ -179,16 +158,6 @@ def is_part(part, master):
             )
             return self
 
-        def topological_sort(self):
-            """:return:  list of nodes in topological order"""
-            return unite_master_parts(
-                list(
-                    nx.algorithms.dag.topological_sort(
-                        nx.DiGraph(self).subgraph(self.nodes_to_show)
-                    )
-                )
-            )
-
         def __add__(self, arg):
             """
             :param arg: either another Diagram or a positive integer.
@@ -256,6 +225,10 @@ def __mul__(self, arg):
             self.nodes_to_show.intersection_update(arg.nodes_to_show)
             return self
 
+        def topo_sort(self):
+            """return nodes in lexicographical topological order"""
+            return topo_sort(self)
+
         def _make_graph(self):
             """
             Make the self.graph - a graph object ready for drawing

datajoint/external.py

@@ -8,7 +8,7 @@
 from .table import Table, FreeTable
 from .heading import Heading
 from .declare import EXTERNAL_TABLE_ROOT
-from . import s3
+from . import s3, errors
 from .utils import safe_write, safe_copy
 
 logger = logging.getLogger(__name__.split(".")[0])
@@ -141,7 +141,12 @@ def _download_buffer(self, external_path):
         if self.spec["protocol"] == "s3":
             return self.s3.get(external_path)
         if self.spec["protocol"] == "file":
-            return Path(external_path).read_bytes()
+            try:
+                return Path(external_path).read_bytes()
+            except FileNotFoundError:
+                raise errors.MissingExternalFile(
+                    f"Missing external file {external_path}"
+                ) from None
         assert False
 
     def _remove_external_file(self, external_path):

datajoint/schemas.py

@@ -2,17 +2,16 @@
 import logging
 import inspect
 import re
-import itertools
 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
 
@@ -413,6 +412,7 @@ def save(self, python_filename=None):
 
         :return: a string containing the body of a complete Python module defining this schema.
         """
+        self.connection.dependencies.load()
         self._assert_exists()
         module_count = itertools.count()
         # add virtual modules for referenced modules with names vmod0, vmod1, ...
@@ -451,10 +451,8 @@ def replace(s):
                 ).replace("\n", "\n    " + indent),
             )
 
-        diagram = Diagram(self)
-        body = "\n\n".join(
-            make_class_definition(table) for table in diagram.topological_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"""',
@@ -480,11 +478,12 @@ def list_tables(self):
 
         :return: A list of table names from the database schema.
         """
+        self.connection.dependencies.load()
         return [
             t
             for d, t in (
                 full_t.replace("`", "").split(".")
-                for full_t in Diagram(self).topological_sort()
+                for full_t in self.connection.dependencies.topo_sort()
             )
             if d == self.database
         ]
@@ -533,7 +532,6 @@ def __init__(
 
 def list_schemas(connection=None):
     """
-
     :param connection: a dj.Connection object
     :return: list of all accessible schemas on the server
     """

datajoint/settings.py

@@ -246,6 +246,11 @@ def __setitem__(self, key, value):
                 self._conf[key] = value
             else:
                 raise DataJointError("Validator for {0:s} did not pass".format(key))
+            valid_logging_levels = {"DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"}
+            if key == "loglevel":
+                if value not in valid_logging_levels:
+                    raise ValueError(f"{'value'} is not a valid logging value")
+                logger.setLevel(value)
 
 
 # Load configuration from file
@@ -270,6 +275,7 @@ def __setitem__(self, key, value):
             "database.password",
             "external.aws_access_key_id",
             "external.aws_secret_access_key",
+            "loglevel",
         ),
         map(
             os.getenv,
@@ -279,6 +285,7 @@ def __setitem__(self, key, value):
                 "DJ_PASS",
                 "DJ_AWS_ACCESS_KEY_ID",
                 "DJ_AWS_SECRET_ACCESS_KEY",
+                "DJ_LOG_LEVEL",
             ),
         ),
     )