fix(#1532): diagram edge weight encodes cardinality, not master-part - #1533
Merged
Conversation
Line weight is binary and encodes cardinality only: thick when the foreign key constitutes the child's entire primary key (1:1), thin when the child has primary-key attributes beyond those the FK contributes (multi-valued) — newly declared or inherited from another FK. penwidth already followed this via multi; remove the misleading master-part conflation in the layout weight and drive it from the same predicate so the two never diverge. Rename-safe: multi compares the child's referencing columns to the child primary key (both child-column space), so a renamed FK that is the child's whole PK is correctly 1:1/thick. Adds a guardrail test (1:1, multi, master-part, renamed-1:1).
dimitri-yatsenko
added
the
bug
dimitri-yatsenko
marked this pull request as ready for review
August 10, 2026 21:41
Member
Author
|
@ttngu207 @MilagrosMarin — part of the diagram-notation set (#1533 edge-weight → #1534 restyle → #1536 no-context fix). New style shown on a real pipeline: https://github.com/datajoint/lcms-demo/tree/viz/diagram-restyle-1534 (datajoint/lcms-demo#3). |
dimitri-yatsenko
added a commit
that referenced
this pull request
Aug 11, 2026
make_mermaid was a stale second renderer on the pre-#1533/#1534 model. Bring it to parity with the Graphviz path: - share the tier palette from _DIAGRAM_THEMES["light"] (drop the old bright #90EE90/#FFB6C1/#ADD8E6 colors) - encode edge thickness as cardinality via linkStyle (thick=1:1, thin=one-to- many) instead of solid/dotted primary-vs-secondary - color renamed FKs in the theme's amber (#C77D3A) - nest a master and its parts in an entity subgraph Documents the three Graphviz features Mermaid can't express (top-right schema label, Helvetica, dimension underline) and leaves dark mode to the host theme. Adds a parity test.
MilagrosMarin
approved these changes
Aug 13, 2026
dimitri-yatsenko
added a commit
that referenced
this pull request
Aug 13, 2026
* fix(#1532): edge weight encodes cardinality, not master-part Line weight is binary and encodes cardinality only: thick when the foreign key constitutes the child's entire primary key (1:1), thin when the child has primary-key attributes beyond those the FK contributes (multi-valued) — newly declared or inherited from another FK. penwidth already followed this via multi; remove the misleading master-part conflation in the layout weight and drive it from the same predicate so the two never diverge. Rename-safe: multi compares the child's referencing columns to the child primary key (both child-column space), so a renamed FK that is the child's whole PK is correctly 1:1/thick. Adds a guardrail test (1:1, multi, master-part, renamed-1:1). * feat(#1532): modernize dj.Diagram rendering style Replace the alpha-blended Graphviz-default look with a readable, modern palette and typography (parts 2 and 4 of #1532): - Tier palette: each tier gets a fill / stroke / text triple (Manual green, Lookup slate, Imported blue, Computed red, Part near-white) in place of the alpha-blended primaries. Shape stays load-bearing and unchanged. - Rounded corners + generous label margins on box tiers; explicit Helvetica font (no more Times fallback). - Renamed-FK edges use a desaturated amber (#C77D3A) instead of vivid #FF8800; ordinary edges a light translucent slate. - Left-to-right, no arrowheads (already the config default). Entity-group clustering (master+parts) and a visual-regression fixture follow. * feat(#1532): master-part entity clustering; drop 'ERD' naming Enclose each master and its parts in a nested, unlabeled entity cluster; part labels drop the master prefix (`C`, not `B.C`) since the enclosure carries membership. Master and its parts share a rank (horizontal = derivation, vertical = containment), except a part that depends on a sibling part, which is left off the rank so the intra-group chain descends. Also rename the diagram test module test_erd.py -> test_diagram.py and its identifiers, and scrub 'ERD' from a code comment: DataJoint diagrams are not ERDs. Adds a test asserting the entity cluster and the prefix-dropped part label. * feat(#1532): master-part entity clustering + drop ERD naming (content) * style(#1532): rounded schema clusters; subtle shaded entity clusters Schema clusters get rounded corners (style rounded,dashed). Master-part entity clusters drop the dashed frame for a quiet rounded shaded background (#F3F5F8, borderless) so the grouping reads subtly and doesn't compete with the schema box. * feat(#1532): dark theme option; matched, higher-contrast edge density Add a diagram color theme option (dj.config display.diagram_theme = light|dark). Refactor make_dot styling into theme-independent structure (_TIER_STRUCTURE) plus per-theme color sets (_DIAGRAM_THEMES): tier fill/stroke/text triples, background, edge colors, and cluster colors. The dark theme uses a deep-slate background with light text and brighter strokes. Edges share one alpha per theme, so a renamed (amber) edge sits at the same visual density as ordinary edges — differing only in hue — and both are given more contrast than the first pass. Also fix a single-underscore config.override example in the docstring (needs double underscore for nested keys). * feat(#1532): adaptive 'auto' theme; brighten dark entity cluster Add theme='auto': render the diagram in light colors and inject a prefers-color-scheme style block so a single SVG adapts to the viewer's light or dark mode (the two palettes are collision-free, so a per-color attribute-selector override is unambiguous). Exposed via Diagram.svg_string(); make_svg/_repr_svg_ use it. make_dot() gains an optional theme override. Also brighten the dark-theme entity-cluster fill so the master-part grouping box reads against the dark background. * style(#1532): Lookup renders as a gray Manual-sized rectangle Match Lookup's box size to Manual (fontsize/size), so a Lookup reads as a gray rectangle the same size as a Manual table rather than a smaller subtle box. Part rendering unchanged (neutral subtle box, per design review). * docs(#1532): note parts keep a box for platform clickability A part inherits its master's tier and has no tier-shape of its own, but it keeps a neutral subtle box (not a tier shape) so platform nodes are clickable targets that open the table. Comment this so it isn't reverted to boxless. * style(#1532): order parts after their master within the shared rank Add invisible ordering edges through each entity's same-rank group (master then parts) so parts are placed below the master in LR and to its right in TB, rather than leaving the within-rank order to Graphviz's heuristic. * fix(#1532): place parts below master (LR) / right of master (TB) The prior ordering edges were being overridden: inside the entity cluster, the master is anchored by its derivation-chain edges and the real master->part FK edge forced the part above (LR). Fix by (a) marking master->part FK edges constraint=false so they don't vote on within-rank order, and (b) adding an invisible ordering edge whose direction depends on rankdir — reversed (part->master) for LR to put parts below, forward (master->part) for TB to put parts to the right. Both recipes verified empirically. * revert(#1532): drop within-cluster part ordering hacks Remove the constraint=false on master->part FK edges and the invisible ordering edges. They ordered simple parts (below in LR / right in TB) but could not do so reliably when a part has its own child part (the child edge + cluster crossing- minimization override the flat-edge order), and the special-casing was brittle. Keep the entity clustering and shared rank; leave within-cluster placement to Graphviz. * test(#1532): add diagram style-regression fixture; robust part detection Add a style-contract test that renders a fixed schema per theme and asserts the palette (per-tier fill/stroke), thick/thin edge weights, entity cluster, dark background, and adaptive prefers-color-scheme block — catching palette/weight/ theme regressions without pinning fragile SVG geometry. Also make master<->part detection in make_dot robust to both node-naming schemes (class 'Master.Part' and raw 'schema.master__part'), using the actual node keys for predecessor lookups instead of reconstructing them — the old code crashed on raw-table-name graphs. * feat(#1532): default diagram theme to 'auto'; sans schema labels Make 'auto' (adaptive light/dark SVG) the default diagram_theme, so rendered diagrams adapt to the viewer's appearance out of the box. Also set the schema cluster label to Helvetica — it previously fell back to Graphviz's Times default while the node labels were already sans. * diagram: place schema name in the top-right corner of its cluster Set labelloc=t, labeljust=r on the schema cluster so the schema/module label sits in the top-right corner rather than top-center. * diagram(mermaid): align make_mermaid with the modernized notation make_mermaid was a stale second renderer on the pre-#1533/#1534 model. Bring it to parity with the Graphviz path: - share the tier palette from _DIAGRAM_THEMES["light"] (drop the old bright #90EE90/#FFB6C1/#ADD8E6 colors) - encode edge thickness as cardinality via linkStyle (thick=1:1, thin=one-to- many) instead of solid/dotted primary-vs-secondary - color renamed FKs in the theme's amber (#C77D3A) - nest a master and its parts in an entity subgraph Documents the three Graphviz features Mermaid can't express (top-right schema label, Helvetica, dimension underline) and leaves dark mode to the host theme. Adds a parity test. * docs(readme): regenerate pipeline figure in dj.Diagram notation (adaptive) Redraw the README example-pipeline figure in the modernized dj.Diagram notation (muted tier palette + colored strokes, Imported/Computed ovals, Manual/Lookup rectangles, thick/thin cardinality edges, master-part entity cluster). Activity is now a Part of Segmentation: cell_mask and trace live on Segmentation.Activity and make() inserts into the part. The SVG is theme-adaptive (auto) — light by default, dark via prefers-color-scheme — so reference the SVG directly and drop the stale raster PNG. * docs(readme): use adaptive dj.Diagram pipeline figure Point the README at the restyled, theme-adaptive images/pipeline.svg (modernized dj.Diagram notation; Activity as a Part of Segmentation with cell_mask/trace on the part and make() inserting into it).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 1 of #1532 — the correctness fix (not style). The style restyle (palette, entity groups, direction, renamed-edge color) will follow as a separate PR.
The rule
Line weight is binary and encodes cardinality only:
penwidth 2) — the foreign key constitutes the child's entire primary key → 1:1.penwidth 0.75) — the child has primary-key attributes beyond those the FK contributes (newly declared, or inherited from another FK) → multi-valued.The visible
penwidthalready followed this (via themultiedge flag). This PR removes the misleading master-part conflation in the layoutweighthint (set_weight(3 if master_part else 1)) — master-part is not a weight; a part almost always adds a key attribute, so its edge is thin under the same rule — and drivesweightfrom the same predicate so the two never diverge.Rename-safe
multi = set(FK child-columns) != set(child PK)compares in child-column space, so a renamed FK that is the child's whole primary key is correctly 1:1/thick. (The parent-PK-vs-child-PK name comparison sketched in the issue would break under renaming; this does not.)Test
tests/integration/test_diagram_edge_weight.pyrenders a diagram and assertspenwidthfor four cases: 1:1 (thick), multi-valued (thin), master→part (thin), and renamed-1:1 (thick). Green on MySQL 8.0 and PostgreSQL 15.Docs companion: datajoint/datajoint-docs#247.