From 069ee376d876560384a824d73a61b176caaae9af Mon Sep 17 00:00:00 2001 From: glopesdev Date: Fri, 2 Aug 2024 12:16:20 +0100 Subject: [PATCH 1/3] Exclude build dependencies when counting inputs --- Bonsai.Editor/GraphModel/WorkflowEditor.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Bonsai.Editor/GraphModel/WorkflowEditor.cs b/Bonsai.Editor/GraphModel/WorkflowEditor.cs index 8453da7a..77e5d866 100644 --- a/Bonsai.Editor/GraphModel/WorkflowEditor.cs +++ b/Bonsai.Editor/GraphModel/WorkflowEditor.cs @@ -1114,9 +1114,10 @@ void ConfigureWorkflowBuilder( CreateGraphNodeType nodeType) { // Estimate number of inputs to the nested node - var inputCount = workflowBuilder.ArgumentRange.LowerBound; - if (nodeType == CreateGraphNodeType.Successor) inputCount = Math.Max(inputCount, selectedNodes.Count()); - else inputCount = Math.Max(inputCount, selectedNodes.Sum(node => workflow.PredecessorEdges(node).Count())); + var inputCount = nodeType == CreateGraphNodeType.Successor + ? selectedNodes.Count(node => !node.Value.IsBuildDependency()) + : selectedNodes.Sum(node => workflow.PredecessorEdges(node).Count(edge => !edge.Item1.Value.IsBuildDependency())); + inputCount = Math.Max(workflowBuilder.ArgumentRange.LowerBound, inputCount); // Limit number of inputs depending on nested operator argument range if (!(workflowBuilder is GroupWorkflowBuilder || workflowBuilder.GetType() == typeof(Defer))) From 97bbdc6d8847ca34fd573eff788c44393b75603f Mon Sep 17 00:00:00 2001 From: glopesdev Date: Wed, 7 Aug 2024 11:30:01 +0100 Subject: [PATCH 2/3] Add infrastructure to test branching workflows --- Bonsai.Core.Tests/TestWorkflow.cs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/Bonsai.Core.Tests/TestWorkflow.cs b/Bonsai.Core.Tests/TestWorkflow.cs index e973c7ed..7353d7a9 100644 --- a/Bonsai.Core.Tests/TestWorkflow.cs +++ b/Bonsai.Core.Tests/TestWorkflow.cs @@ -12,9 +12,13 @@ public TestWorkflow() { } - private TestWorkflow(ExpressionBuilderGraph workflow, Node cursor) + private TestWorkflow( + ExpressionBuilderGraph workflow, + Node cursor, + int argumentIndex = 0) { Workflow = workflow ?? throw new ArgumentNullException(nameof(workflow)); + ArgumentIndex = argumentIndex; Cursor = cursor; } @@ -22,6 +26,8 @@ private TestWorkflow(ExpressionBuilderGraph workflow, Node Cursor { get; } + public int ArgumentIndex { get; } + public TestWorkflow ResetCursor() { if (Cursor != null) @@ -41,7 +47,17 @@ public TestWorkflow Append(ExpressionBuilder builder) var node = Workflow.Add(builder); if (Cursor != null) Workflow.AddEdge(Cursor, node, new ExpressionBuilderArgument()); - return new TestWorkflow(Workflow, node); + return new TestWorkflow(Workflow, node, argumentIndex: 1); + } + + public TestWorkflow AddArguments(params TestWorkflow[] arguments) + { + var argumentIndex = ArgumentIndex; + for (int i = 0; i < arguments.Length; i++) + { + Workflow.AddEdge(arguments[i].Cursor, Cursor, new ExpressionBuilderArgument(argumentIndex++)); + } + return new TestWorkflow(Workflow, Cursor, argumentIndex); } public TestWorkflow AppendCombinator(TCombinator combinator) where TCombinator : new() @@ -81,6 +97,11 @@ public TestWorkflow AppendPropertyMapping(params string[] propertyNames) return Append(mappingBuilder); } + public TestWorkflow AppendBranch(Func selector) + { + return selector(this); + } + public TestWorkflow AppendNested( Func selector, Func constructor) From 7a875e3f51d85fc02f35b29dbe95f1b46da7092a Mon Sep 17 00:00:00 2001 From: glopesdev Date: Wed, 7 Aug 2024 19:23:43 +0100 Subject: [PATCH 3/3] Add regression test for replacing graph node --- .../Bonsai.Editor.Tests.csproj | 1 + Bonsai.Editor.Tests/WorkflowEditorTests.cs | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/Bonsai.Editor.Tests/Bonsai.Editor.Tests.csproj b/Bonsai.Editor.Tests/Bonsai.Editor.Tests.csproj index 1f6d3148..e20eeafd 100644 --- a/Bonsai.Editor.Tests/Bonsai.Editor.Tests.csproj +++ b/Bonsai.Editor.Tests/Bonsai.Editor.Tests.csproj @@ -17,6 +17,7 @@ + \ No newline at end of file diff --git a/Bonsai.Editor.Tests/WorkflowEditorTests.cs b/Bonsai.Editor.Tests/WorkflowEditorTests.cs index 4b4930d0..1a762e84 100644 --- a/Bonsai.Editor.Tests/WorkflowEditorTests.cs +++ b/Bonsai.Editor.Tests/WorkflowEditorTests.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; using System.Xml; +using Bonsai.Core.Tests; using Bonsai.Dag; using Bonsai.Editor.GraphModel; using Bonsai.Expressions; @@ -47,6 +48,12 @@ static string ToString(IEnumerable sequence) ExpressionBuilderGraph workflow = null, MockGraphView graphView = null) { + if (workflow != null) + { + // Workflows must be topologically sorted to ensure all editor operations are reversible + workflow.InsertRange(0, workflow.TopologicalSort()); + } + graphView ??= new MockGraphView(workflow); var editor = new WorkflowEditor(graphView.ServiceProvider, graphView); editor.UpdateLayout.Subscribe(graphView.UpdateGraphLayout); @@ -219,6 +226,35 @@ public void CreateAnnotation_EmptySelection_InsertAfterClosestRoot() Assert.AreEqual(expected: editor.Workflow.Count - 1, editor.FindNode(annotationBuilder).Index); assertIsReversible(); } + + [TestMethod] + public void ReplaceGraphNode_SingleInputWithVisualizerMapping_GroupWorkflowHasSingleSourceNode() + { + // related to https://github.com/bonsai-rx/bonsai/issues/1792 + var workflow = new TestWorkflow() + .AppendValue(0) + .AppendBranch(source => source + .AppendSubject("P") + .AddArguments(source.Append(new VisualizerMappingBuilder()))) + .Workflow + .ToInspectableGraph(); + + var (editor, assertIsReversible) = CreateMockEditor(workflow); + var targetNode = editor.FindNode("P"); + editor.ReplaceGraphNode( + targetNode, + typeof(GroupWorkflowBuilder).AssemblyQualifiedName, + ElementCategory.Nested, + arguments: "N"); + Assert.AreEqual(expected: 3, workflow.Count); + + var groupNode = editor.FindNode("N"); + var groupBuilder = ExpressionBuilder.Unwrap(groupNode?.Value) as GroupWorkflowBuilder; + Assert.IsInstanceOfType(groupBuilder, typeof(GroupWorkflowBuilder)); + Assert.AreEqual(expected: 2, groupBuilder.Workflow.Count); + Assert.IsInstanceOfType(ExpressionBuilder.Unwrap(groupBuilder.Workflow[1].Value), typeof(WorkflowOutputBuilder)); + assertIsReversible(); + } } static class WorkflowEditorHelper