Address Apollo review deferred items (S2, I4, S5)
Some checks failed
Create Examples / build (ubuntu-22.04, 3.7) (push) Has been cancelled
Create Examples / build (ubuntu-22.04, 3.8) (push) Has been cancelled
Create Examples / build (ubuntu-latest, 3.10) (push) Has been cancelled
Create Examples / build (ubuntu-latest, 3.11) (push) Has been cancelled
Create Examples / build (ubuntu-latest, 3.12) (push) Has been cancelled
Create Examples / build (ubuntu-latest, 3.9) (push) Has been cancelled

S2: Replace type() == list with isinstance() for list-to-dict conversion.

I4: Fix duplicate GraphViz port names when a pin appears in multiple
shorts. Port names now encode short column index (p{idx}j{col}) so
each cell in the shorts grid has a unique port. Edge generation in
gv_connector_shorts() updated to match.

S5: Add rendering tests for multi-pin loop edge chains and multi-short
port name uniqueness.
This commit is contained in:
Ryan Malloy 2026-02-13 03:12:14 -07:00
parent 6198f7352a
commit 17ef5806f1
3 changed files with 68 additions and 11 deletions

View File

@ -470,7 +470,7 @@ class Connector(TopLevelGraphicalComponent):
self.show_pincount = self.style != "simple" self.show_pincount = self.style != "simple"
# Convert short List to Short Dict # Convert short List to Short Dict
if type(self.shorts) == list: if isinstance(self.shorts, list):
self.shorts_hide_lable = True self.shorts_hide_lable = True
shDict = dict() shDict = dict()
for shIndex in range(0, len(self.shorts)): for shIndex in range(0, len(self.shorts)):
@ -479,7 +479,7 @@ class Connector(TopLevelGraphicalComponent):
self.shorts = shDict self.shorts = shDict
# Convert loop List to loop Dict # Convert loop List to loop Dict
if type(self.loops) == list: if isinstance(self.loops, list):
loDict = dict() loDict = dict()
for loIndex in range(0, len(self.loops)): for loIndex in range(0, len(self.loops)):
key = "AutoLO" + str(loIndex) key = "AutoLO" + str(loIndex)

View File

@ -303,10 +303,12 @@ def gv_pin_table(component) -> Table:
def gv_short_row_part(pin, connector) -> List: def gv_short_row_part(pin, connector) -> List:
short_row = [] # Td("ADA"), Td("DAD") short_row = []
for short, shPins in connector.shorts.items(): for sh_col, (short, shPins) in enumerate(connector.shorts.items()):
if pin.id in shPins: if pin.id in shPins:
short_row.append(Td("", port=f"p{pin.index+1}j")) # Port name encodes pin index + short column to stay unique
# when the same pin appears in multiple shorts.
short_row.append(Td("", port=f"p{pin.index+1}j{sh_col}"))
else: else:
short_row.append(Td("")) short_row.append(Td(""))
return short_row if len(short_row) > 0 else None return short_row if len(short_row) > 0 else None
@ -358,7 +360,7 @@ def gv_connector_loops(connector: Connector) -> List:
def gv_connector_shorts(connector: Connector) -> List: def gv_connector_shorts(connector: Connector) -> List:
short_edges = [] short_edges = []
for short, shPins in connector.shorts.items(): for sh_col, (short, shPins) in enumerate(connector.shorts.items()):
comp = getAddCompFromRef(short, connector) comp = getAddCompFromRef(short, connector)
shColor = "#FFFFFF:#000000:#FFFFFF" shColor = "#FFFFFF:#000000:#FFFFFF"
if comp != None and comp.color != None: if comp != None and comp.color != None:
@ -366,10 +368,11 @@ def gv_connector_shorts(connector: Connector) -> List:
for i in range(1, len(shPins)): for i in range(1, len(shPins)):
# Convert pin IDs to 1-based port indices (same fix as loops). # Convert pin IDs to 1-based port indices (same fix as loops).
# Port name includes short column index for uniqueness.
idx_prev = connector.pin_objects[shPins[i - 1]].index + 1 idx_prev = connector.pin_objects[shPins[i - 1]].index + 1
idx_curr = connector.pin_objects[shPins[i]].index + 1 idx_curr = connector.pin_objects[shPins[i]].index + 1
head = f"{connector.designator}:p{idx_prev}j:c" head = f"{connector.designator}:p{idx_prev}j{sh_col}:c"
tail = f"{connector.designator}:p{idx_curr}j:c" tail = f"{connector.designator}:p{idx_curr}j{sh_col}:c"
short_edges.append((head, tail, shColor)) short_edges.append((head, tail, shColor))
return short_edges return short_edges

View File

@ -334,12 +334,13 @@ class TestRendering:
graph = harness.create_graph() graph = harness.create_graph()
gv = graph.source gv = graph.source
# Short should reference p1j and p2j (indices), not p10j and p20j # Short should reference p1j0 and p2j0 (index + column 0),
short_edges = re.findall(r"X1:p(\d+)j:c -- X1:p(\d+)j:c", gv) # not p10j0 and p20j0 (pin IDs)
short_edges = re.findall(r"X1:p(\d+)j\d+:c -- X1:p(\d+)j\d+:c", gv)
assert len(short_edges) >= 1, f"Expected short edge, found none in:\n{gv}" assert len(short_edges) >= 1, f"Expected short edge, found none in:\n{gv}"
idx_a, idx_b = short_edges[0] idx_a, idx_b = short_edges[0]
assert idx_a == "1" and idx_b == "2", ( assert idx_a == "1" and idx_b == "2", (
f"Short ports should be p1j/p2j (indices), got p{idx_a}j/p{idx_b}j" f"Short ports should be p1/p2 (indices), got p{idx_a}/p{idx_b}"
) )
def test_sequential_pins_still_work(self): def test_sequential_pins_still_work(self):
@ -357,6 +358,59 @@ class TestRendering:
assert ":p1" in gv assert ":p1" in gv
assert ":p2" in gv assert ":p2" in gv
def test_multi_pin_loop_renders_chain(self):
"""S5: >2 pin loop produces correct edge chain (p1->p2, p2->p3)."""
import re
harness = self._make_harness()
harness.add_connector(
"X1",
pins=[10, 20, 30, 40],
pinlabels=["A", "B", "C", "D"],
loops=[["A", "B", "C"]],
)
harness.add_cable("W1", wirecount=1, colors=["BK"])
harness.connect("X1", "D", "W1", 1, None, None)
graph = harness.create_graph()
gv = graph.source
loop_edges = re.findall(r"X1:p(\d+)\w:\w -- X1:p(\d+)\w:\w", gv)
# 3-pin loop should produce 2 edges: p1->p2 and p2->p3
assert len(loop_edges) == 2, (
f"Expected 2 loop edges for 3-pin loop, got {len(loop_edges)}"
)
assert loop_edges[0] == ("1", "2")
assert loop_edges[1] == ("2", "3")
def test_pin_in_multiple_shorts_unique_ports(self):
"""I4: pin in two shorts gets unique port names per column."""
import re
harness = self._make_harness()
harness.add_connector(
"X1",
pins=[1, 2, 3, 4],
shorts={"SH1": [1, 2], "SH2": [1, 3]},
)
harness.add_cable("W1", wirecount=1, colors=["BK"])
harness.connect("X1", 4, "W1", 1, None, None)
graph = harness.create_graph()
gv = graph.source
# SH1 edges should use column 0 (j0), SH2 should use column 1 (j1)
sh1_edges = re.findall(r"X1:p(\d+)j0:c -- X1:p(\d+)j0:c", gv)
sh2_edges = re.findall(r"X1:p(\d+)j1:c -- X1:p(\d+)j1:c", gv)
assert len(sh1_edges) >= 1, f"Expected SH1 edge (j0), found none in:\n{gv}"
assert len(sh2_edges) >= 1, f"Expected SH2 edge (j1), found none in:\n{gv}"
# SH1: pin 1 (idx 1) -> pin 2 (idx 2)
assert sh1_edges[0] == ("1", "2")
# SH2: pin 1 (idx 1) -> pin 3 (idx 3)
assert sh2_edges[0] == ("1", "3")
# Verify no duplicate port names in the HTML table
port_names = re.findall(r'PORT="(p\d+j\d+)"', gv)
assert len(port_names) == len(set(port_names)), (
f"Duplicate port names found: {port_names}"
)
# --- Harness.connect() delegation --- # --- Harness.connect() delegation ---