Skip to content

Commit ba1a237

Browse files
fix: collapse chaining for multiple collapsed diagrams
Fixed bug where A.collapse() + B.collapse() + C.collapse() only collapsed the last diagram. The issue was: 1. _apply_collapse returned early when _explicit_nodes was empty 2. Combined diagrams lost track of which nodes came from collapsed sources Changes: - Remove early return when _explicit_nodes is empty - Track explicit nodes properly through chained + operations - Fresh non-collapsed diagrams add all nodes to explicit - Combined diagrams only add their existing explicit nodes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent c3c4c0f commit ba1a237

File tree

1 file changed

+20
-11
lines changed

1 file changed

+20
-11
lines changed

src/datajoint/diagram.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -232,17 +232,26 @@ def __add__(self, arg) -> "Diagram":
232232
result.nodes_to_show.update(arg.nodes_to_show)
233233
# Merge contexts for class name lookups
234234
result.context = {**result.context, **arg.context}
235-
# Handle collapse: nodes from non-collapsed diagrams are explicit (expanded)
236-
if not self._is_collapsed:
235+
# Handle collapse: track which nodes should be explicit (expanded)
236+
# - Always preserve existing _explicit_nodes from both sides
237+
# - For a fresh (non-combined) non-collapsed diagram, add all its nodes to explicit
238+
# - A fresh diagram has empty _explicit_nodes and _is_collapsed=False
239+
# This ensures "expanded wins" and chained collapsed diagrams stay collapsed
240+
result._explicit_nodes = set()
241+
# Add self's explicit nodes
242+
result._explicit_nodes.update(self._explicit_nodes)
243+
# If self is a fresh non-collapsed diagram (not combined, not marked collapsed),
244+
# treat all its nodes as explicit
245+
if not self._is_collapsed and not self._explicit_nodes:
237246
result._explicit_nodes.update(self.nodes_to_show)
238-
else:
239-
result._explicit_nodes.update(self._explicit_nodes)
240-
if not arg._is_collapsed:
247+
# Add arg's explicit nodes
248+
result._explicit_nodes.update(arg._explicit_nodes)
249+
# If arg is a fresh non-collapsed diagram, treat all its nodes as explicit
250+
if not arg._is_collapsed and not arg._explicit_nodes:
241251
result._explicit_nodes.update(arg.nodes_to_show)
242-
else:
243-
result._explicit_nodes.update(arg._explicit_nodes)
244-
# Result is not collapsed (it's a combination)
245-
result._is_collapsed = False
252+
# Result is "collapsed" if BOTH operands were collapsed (no explicit nodes added)
253+
# This allows chained collapsed diagrams to stay collapsed: A.collapse() + B.collapse() + C.collapse()
254+
result._is_collapsed = self._is_collapsed and arg._is_collapsed
246255
except AttributeError:
247256
try:
248257
result.nodes_to_show.add(arg.full_table_name)
@@ -377,8 +386,8 @@ def _apply_collapse(self, graph: nx.DiGraph) -> tuple[nx.DiGraph, dict[str, str]
377386
valid_nodes = self.nodes_to_show.intersection(set(self.nodes()))
378387
valid_explicit = self._explicit_nodes.intersection(set(self.nodes()))
379388

380-
if not valid_explicit or valid_explicit == valid_nodes:
381-
# No collapse needed
389+
if valid_explicit == valid_nodes:
390+
# All nodes are explicit (expanded) - no collapse needed
382391
return graph, {}
383392

384393
# Map full_table_names to class_names

0 commit comments

Comments
 (0)