Skip to content

refactor(graph): migrate Dependencies & Diagram to nx.MultiDiGraph (#1492)#1508

Open
dimitri-yatsenko wants to merge 3 commits into
masterfrom
fix/1492-multidigraph
Open

refactor(graph): migrate Dependencies & Diagram to nx.MultiDiGraph (#1492)#1508
dimitri-yatsenko wants to merge 3 commits into
masterfrom
fix/1492-multidigraph

Conversation

@dimitri-yatsenko

Copy link
Copy Markdown
Member

Resolves #1492.

Migrates Dependencies and Diagram from nx.DiGraph to nx.MultiDiGraph, retiring the integer "alias node" workaround used to represent renamed / multiple foreign keys between the same pair of tables. A renamed FK is now a first-class parallel edge keyed by (parent, child, key) — no synthetic intermediate node.

Why

A plain DiGraph allows only one edge per ordered node pair, so a renamed FK was split into parent → "N" → child through an integer alias node (dependencies.py), and ~14 .isdigit() special-cases across diagram.py (plus 5 in table.py) existed solely to see through those nodes. MultiDiGraph represents parallel edges natively, so the whole workaround is unnecessary — it was a historical design choice (the class predates our use of multigraphs), not a NetworkX limitation.

What changed

Graph model

  • Dependencies(nx.MultiDiGraph), Diagram(nx.MultiDiGraph).
  • FK edge creation collapses to a single add_edge(parent, child, **props) (no alias node, no _node_alias_count/itertools).
  • topo_sort drops its alias-collapse pass; cascade()/trace() rebuild via MultiDiGraph (a DiGraph rebuild would silently drop parallel edges).

Edge-data addressing — every single-edge access moved to multigraph semantics:

  • Both propagation walkers (_propagate_restrictions, _propagate_restrictions_upstream) merged their alias/direct branches into one loop over parallel edges, with the dedup set keyed on the (u, v, key) edge key so parallel FKs aren't dropped.
  • _find_real_edge_props (alias-traversing) → _edge_props (direct get_edge_data lookup).
  • _encapsulate_edge_attributes writes per-edge via (u, v, key).

Full multi-edge public API

  • dependencies.parents() / children() now return a parallel-safe list[(node, props)] instead of a name-keyed dict that collapsed multiple FKs from the same parent. Table.parents()/children() consume the list (the .isdigit() unwrap is gone); descendants()/ancestors()/parts() drop their alias filters.

Rendering (renamed FK, formerly the orange alias dot, is now an edge)

  • make_dot: renamed FKs draw as #FF8800 edges with the column renames in an edge tooltip (hover in SVG); reads each parallel edge's props directly off the pydot edge.
  • make_mermaid: renamed FK edges recolored via linkStyle … stroke:#FF8800.
  • _AliasNode tier removed (user_tables.py); display graph is a MultiDiGraph so parallel FKs survive to the renderer.

Verification

Ran the full tests/ suite under the pixi test environment (MySQL 8.0 + PostgreSQL 15 via testcontainers):

  • Established a green baseline (95 passed) on the target graph/cascade/trace/FK suite before touching code, then reproduced 95 passed after the refactor — behavior-identical.
  • Renamed-FK behavior explicitly covered and green: test_aliased_fk, test_trace_renamed_fk, test_cascade_delete_with_renamed_attrs, test_cascade_part_of_part_renamed_fk, test_upward_u3_nonprimary_master_fk; render smoke tests on the aliased schema_advanced (test_repr_svg, test_make_image) pass.
  • Full tests/ suite: 949 passed, 10 skipped, 0 failures (both backends) — no downstream regressions.

Notes / follow-ups

  • In a partially collapsed diagram, parallel edges between two still-expanded tables are deduped to one arrow by _apply_collapse; the common fully-expanded diagram preserves them. Can refine later if needed.
  • Opened as a draft — this is a broad, cross-cutting v2.4 change; worth careful review of the propagation edge-key semantics in particular.

…1492)

Retire the integer alias-node workaround for renamed/parallel foreign keys.
A plain DiGraph allows one edge per node pair, so a renamed FK was split
through an integer alias node, with ~14 .isdigit() special-cases (diagram.py)
plus 5 in table.py to see through them. MultiDiGraph represents parallel edges
natively (keyed by (u, v, key)), so the workaround is removed entirely.

- Dependencies/Diagram now subclass nx.MultiDiGraph; FK edge creation is a
  single add_edge; topo_sort drops alias-collapse; cascade()/trace() rebuild
  via MultiDiGraph.
- Propagation walkers merged their alias/direct branches into one loop over
  parallel edges, deduping on the (u, v, key) edge key.
- _find_real_edge_props -> _edge_props (direct lookup); _encapsulate_edge_
  attributes writes per (u, v, key).
- dependencies.parents()/children() return parallel-safe [(node, props)];
  Table.parents/children/descendants/ancestors/parts drop alias handling.
- Rendering: renamed FKs draw as #FF8800 edges with a rename tooltip (dot) and
  linkStyle recolor (mermaid); _AliasNode tier removed.

Verified: full tests/ suite 949 passed / 10 skipped / 0 failed on MySQL 8.0
and PostgreSQL 15 (testcontainers); baseline was identical before the change.
@dimitri-yatsenko dimitri-yatsenko added the enhancement Indicates new improvements label Jul 17, 2026
@dimitri-yatsenko
dimitri-yatsenko marked this pull request as ready for review July 17, 2026 22:57
@dimitri-yatsenko dimitri-yatsenko added this to the v2.3.2 milestone Jul 17, 2026
On a MultiDiGraph, Diagram.__add__ used add_edges_from(arg.edges(data=True)),
which assigns a fresh parallel-edge key per call — so every FK shared between
two combined diagrams (they share the dependency graph) was duplicated, and a
single foreign key rendered as multiple overlapping edges. Merge edges by their
(u, v, key) instead, skipping ones already present.

Adds tests/integration/test_erd.py::test_diagram_operators_no_duplicate_edges
asserting +, +N, -N, *, add_parts, and from_sequence all preserve the
dependency graph's edge multiplicity.
…llapse

_apply_collapse merged edges globally via has_edge(u, v), so in a partially
collapsed diagram two renamed FKs between the same pair of still-expanded tables
(e.g. Person -> Marriage spouse1/spouse2) collapsed into a single arrow, dropping
a real FK from the picture. Now edges touching a collapsed schema node still
merge to one arrow per pair, but edges between two expanded nodes are kept as
distinct parallel edges keyed by (u, v, key).
@dimitri-yatsenko

Copy link
Copy Markdown
Member Author

Paired docs PR: datajoint/datajoint-docs#215 — updates the diagram-notation docs (renamed FKs render as orange edges with a rename tooltip instead of alias-node dots) and adds a renamed-primary example showing the line-style semantics are retained.

The docs PR re-executed its notebooks against this branch, so it is gated on this shipping in v2.3.2 and must not merge until the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Indicates new improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate dependency & Diagram graphs to nx.MultiDiGraph (retire alias-node workaround)

1 participant