diff --git a/src/datajoint/diagram.py b/src/datajoint/diagram.py index 6ff60d061..59670bbee 100644 --- a/src/datajoint/diagram.py +++ b/src/datajoint/diagram.py @@ -1206,8 +1206,12 @@ def _make_graph(self) -> nx.MultiDiGraph: # primary-key attribute of their own beyond what they inherit through # foreign keys. These are drawn with an underlined label. ("Schema # dimension" / "axis" is the documented term for such a table.) - # Filter nodes_to_show to only include nodes that exist in the graph - valid_nodes = self.nodes_to_show.intersection(set(self.nodes())) + # Filter nodes_to_show to only include nodes that exist in the graph. + # Sorted, because the rendered output follows this order: table names + # are strings, whose hashes are salted per process, so iterating the set + # would emit nodes, edges and clusters in a different order on every + # run (#1551). + valid_nodes = sorted(self.nodes_to_show.intersection(set(self.nodes()))) for name in valid_nodes: foreign_attributes = set( attr for p in self.in_edges(name, data=True) for attr in p[2]["attr_map"] if p[2]["primary"] @@ -1215,12 +1219,25 @@ def _make_graph(self) -> nx.MultiDiGraph: self.nodes[name]["introduces_dimension"] = ( "primary_key" in self.nodes[name] and foreign_attributes < self.nodes[name]["primary_key"] ) - # construct subgraph and rename nodes to class names. A MultiDiGraph is - # used so parallel FKs between the same pair of tables (e.g. a renamed - # FK alongside a direct one) survive to the renderer as distinct edges. - # Subgraph a plain MultiDiGraph copy — subgraphing/copying Diagram itself - # would try to instantiate Diagram() (which requires a `source`). - graph = nx.MultiDiGraph(nx.MultiDiGraph(self).subgraph(valid_nodes)) + # Construct the drawing graph and rename nodes to class names. A + # MultiDiGraph is used so parallel FKs between the same pair of tables + # (e.g. a renamed FK alongside a direct one) survive to the renderer as + # distinct edges. It is built here in sorted order rather than with + # subgraph(): a subgraph view holding fewer than half of the nodes + # iterates them in set order. to_pydot() writes every node attribute + # into the dot source with str(), so the primary-key set is stored sorted. + graph = nx.MultiDiGraph() + for name in valid_nodes: + attrs = dict(self.nodes[name]) + if "primary_key" in attrs: + attrs["primary_key"] = sorted(attrs["primary_key"]) + graph.add_node(name, **attrs) + graph.add_edges_from( + sorted( + ((u, v, k, d) for u, v, k, d in self.edges(valid_nodes, keys=True, data=True) if v in graph), + key=lambda edge: (edge[0], edge[1], str(edge[2])), + ) + ) nx.set_node_attributes(graph, name="node_type", values={n: _get_tier(n) for n in graph}) # relabel nodes to class names mapping = {node: lookup_class_name(node, self.context) or node for node in graph.nodes()} @@ -1270,7 +1287,7 @@ def _apply_collapse(self, graph: nx.MultiDiGraph) -> tuple[nx.MultiDiGraph, dict # Group collapsed nodes by schema collapsed_by_schema = {} # schema_name -> list of class_names - for class_name in nodes_to_collapse: + for class_name in sorted(nodes_to_collapse): full_name = class_to_full.get(class_name) if full_name: schema_name, _ = self._connection.adapter.split_full_table_name(full_name) @@ -1501,7 +1518,7 @@ def make_dot(self, theme=None): schema_map = {} # class_name -> schema_name schema_modules = {} # schema_name -> set of module names - for full_name in self.nodes_to_show: + for full_name in sorted(self.nodes_to_show): schema_name, _ = self._connection.adapter.split_full_table_name(full_name) if schema_name: class_name = lookup_class_name(full_name, self.context) or full_name @@ -1745,9 +1762,11 @@ def _master_of(part_stripped): fontname="Helvetica", # schema label in a sans font, not Graphviz's Times default ) node_by_name = {n.get_name().strip('"'): n for n in nodes} - # masters in this schema that have at least one part present + # masters in this schema that have at least one part present, + # in sorted order so the entity clusters are emitted in a fixed + # order (#1551) masters_here = {} - for pn in part_names: + for pn in sorted(part_names): mn = part_master[pn] if pn in node_by_name and mn in node_by_name: masters_here.setdefault(mn, []).append(pn) @@ -1870,7 +1889,7 @@ def make_mermaid(self) -> str: schema_map = {} # class_name -> schema_name schema_modules = {} # schema_name -> set of module names - for full_name in self.nodes_to_show: + for full_name in sorted(self.nodes_to_show): schema_name, _ = self._connection.adapter.split_full_table_name(full_name) if schema_name: class_name = lookup_class_name(full_name, self.context) or full_name diff --git a/tests/integration/test_diagram_reproducible.py b/tests/integration/test_diagram_reproducible.py new file mode 100644 index 000000000..3eb139941 --- /dev/null +++ b/tests/integration/test_diagram_reproducible.py @@ -0,0 +1,202 @@ +""" +Byte-reproducibility of rendered diagrams across processes (#1551). + +Python salts ``str.__hash__`` per process (PEP 456), so iterating a set of table +names yields a different order in every interpreter. Wherever such an order +reaches the renderer, two identical runs emit nodes, edges and clusters in a +different sequence and the output differs byte for byte, although the picture +is the same. That defeats committing generated figures and checking them for +drift. An in-process test cannot catch this, so the same diagram is rendered in +separate interpreters with different ``PYTHONHASHSEED`` values and compared. +""" + +import json +import os +import subprocess +import sys +import time + +import pytest + +import datajoint as dj + +# Declares a pipeline with several master-part groups and renders it three ways: +# the whole schema, a neighborhood that holds fewer than half of the loaded +# tables (networkx then iterates its subgraph in set order), and a partly +# collapsed diagram. Writes the dot source, SVG and Mermaid of each as JSON. +_SCRIPT = ''' +import json +import sys + +import datajoint as dj + +cfg = json.loads(sys.argv[1]) +dj.config["database.backend"] = cfg["backend"] +dj.config["database.host"] = cfg["host"] +dj.config["database.port"] = cfg["port"] +dj.config["database.user"] = cfg["user"] +dj.config["database.password"] = cfg["password"] + +schema = dj.Schema(cfg["schema"]) + + +@schema +class Subject(dj.Manual): + definition = """ + subject_id : int32 + """ + + +@schema +class Rig(dj.Lookup): + definition = """ + rig : varchar(16) + """ + + +@schema +class Session(dj.Manual): + definition = """ + -> Subject + session_id : int32 + --- + -> Rig + """ + + class Note(dj.Part): + definition = """ + -> master + note_id : int32 + """ + + class Stimulus(dj.Part): + definition = """ + -> master + stim_id : int32 + """ + + +@schema +class Scan(dj.Imported): + definition = """ + -> Session + """ + + class Field(dj.Part): + definition = """ + -> master + field_id : int32 + """ + + class Channel(dj.Part): + definition = """ + -> master + channel : int32 + """ + + +@schema +class Segmentation(dj.Computed): + definition = """ + -> Scan + """ + + class Mask(dj.Part): + definition = """ + -> master + -> Scan.Field + mask_id : int32 + """ + + +@schema +class Fluorescence(dj.Computed): + definition = """ + -> Segmentation + """ + + class Trace(dj.Part): + definition = """ + -> master + -> Segmentation.Mask + """ + + +@schema +class Pairing(dj.Manual): + definition = """ + -> Session + -> Session.proj(other_subject_id="subject_id", other_session_id="session_id") + """ + + +diagrams = { + "schema": dj.Diagram(schema), + "neighborhood": dj.Diagram(Scan) + 1, + "collapsed": dj.Diagram(schema).collapse() + dj.Diagram(Segmentation), +} +rendered = { + name: {"dot": d.make_dot().to_string(), "svg": d.svg_string(), "mermaid": d.make_mermaid()} + for name, d in diagrams.items() +} +with open(cfg["out"], "w") as f: + json.dump(rendered, f) +''' + +# Seeds chosen so that, before the fix, set iteration order differed between them. +_SEEDS = ("0", "1", "12345") + + +@pytest.fixture(scope="function") +def schema_name_by_backend(connection_by_backend, db_creds_by_backend): + backend = db_creds_by_backend["backend"] + test_id = str(int(time.time() * 1000))[-8:] + schema_name = f"djtest_repro_{backend}_{test_id}"[:64] + quoted = connection_by_backend.adapter.quote_identifier(schema_name) + if connection_by_backend.is_connected: + try: + connection_by_backend.query(f"DROP DATABASE IF EXISTS {quoted}") + except Exception: + pass + dj.Schema(schema_name, connection=connection_by_backend) + yield schema_name + if connection_by_backend.is_connected: + try: + connection_by_backend.query(f"DROP DATABASE IF EXISTS {quoted}") + except Exception: + pass + + +def _render(seed, cfg, tmp_path): + out = tmp_path / f"render_{seed}.json" + proc = subprocess.run( + [sys.executable, "-c", _SCRIPT, json.dumps(dict(cfg, out=str(out)))], + env=dict(os.environ, PYTHONHASHSEED=seed), + cwd=tmp_path, + capture_output=True, + text=True, + timeout=300, + ) + assert proc.returncode == 0, f"render with PYTHONHASHSEED={seed} failed:\n{proc.stderr}" + return json.loads(out.read_text()) + + +def test_render_is_independent_of_hash_seed(schema_name_by_backend, db_creds_by_backend, tmp_path): + host, port = db_creds_by_backend["host"].rsplit(":", 1) + cfg = dict( + backend=db_creds_by_backend["backend"], + host=host, + port=int(port), + user=db_creds_by_backend["user"], + password=db_creds_by_backend["password"], + schema=schema_name_by_backend, + ) + reference, *others = (_render(seed, cfg, tmp_path) for seed in _SEEDS) + mismatches = [ + f"{name} diagram, {fmt} output, PYTHONHASHSEED={_SEEDS[0]} vs {seed}" + for seed, rendered in zip(_SEEDS[1:], others) + for name, outputs in reference.items() + for fmt, text in outputs.items() + if rendered[name][fmt] != text + ] + assert not mismatches, "rendering depends on the hash seed:\n" + "\n".join(mismatches)