From c09a0d2327cd58f4118365d64ebd906300ad550c Mon Sep 17 00:00:00 2001 From: Yann Lanthony Date: Thu, 14 Nov 2024 19:11:08 +0100 Subject: [PATCH 1/5] [core] Add 'strictCompatibility' mode for loadGraph function Provide a way to control how to handle node compatibility issues when loading a Graph with the `strictCompatibility` parameter. Introduce a new error type, GraphCompatibilityError, to be raised when loading a Graph with compatibility issues with strictCompatibility enabled. --- meshroom/core/exception.py | 15 +++++++++++++++ meshroom/core/graph.py | 20 ++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/meshroom/core/exception.py b/meshroom/core/exception.py index 9bffd49afb..2365f8e21a 100644 --- a/meshroom/core/exception.py +++ b/meshroom/core/exception.py @@ -12,6 +12,21 @@ class GraphException(MeshroomException): pass +class GraphCompatibilityError(GraphException): + """ + Raised when node compatibility issues occur when loading a graph. + + Args: + filepath: The path to the file that caused the error. + issues: A dictionnary of node names and their respective compatibility issues. + """ + def __init__(self, filepath, issues: dict[str, str]) -> None: + self.filepath = filepath + self.issues = issues + msg = f"Compatibility issues found when loading {self.filepath}: {self.issues}" + super().__init__(msg) + + class UnknownNodeTypeError(GraphException): """ Raised when asked to create a unknown node type. diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index e278ae33e9..ebe2de1ba5 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -15,7 +15,7 @@ from meshroom.common import BaseObject, DictModel, Slot, Signal, Property from meshroom.core import Version from meshroom.core.attribute import Attribute, ListAttribute, GroupAttribute -from meshroom.core.exception import StopGraphVisit, StopBranchVisit +from meshroom.core.exception import GraphCompatibilityError, StopGraphVisit, StopBranchVisit from meshroom.core.node import nodeFactory, Status, Node, CompatibilityNode # Replace default encoder to support Enums @@ -1633,11 +1633,27 @@ def setVerbose(self, v): canComputeLeaves = Property(bool, lambda self: self._canComputeLeaves, notify=canComputeLeavesChanged) -def loadGraph(filepath): +def loadGraph(filepath, strictCompatibility: bool = False) -> Graph: """ + Load a Graph from a Meshroom Graph (.mg) file. + + Args: + filepath: The path to the Meshroom Graph file. + strictCompatibility: If True, raise a GraphCompatibilityError if the loaded Graph has node compatibility issues. + + Returns: + Graph: The loaded Graph instance. + + Raises: + GraphCompatibilityError: If the Graph has node compatibility issues and `strictCompatibility` is False. """ graph = Graph("") graph.load(filepath) + + compatibilityIssues = len(graph.compatibilityNodes) > 0 + if compatibilityIssues and strictCompatibility: + raise GraphCompatibilityError(filepath, {n.name: str(n.issue) for n in graph.compatibilityNodes}) + graph.update() return graph From 3218c41bdca777f928ba7d53abdbda74399b856b Mon Sep 17 00:00:00 2001 From: Yann Lanthony Date: Thu, 14 Nov 2024 19:11:08 +0100 Subject: [PATCH 2/5] [tests] Add strictCompatibility graph loading test --- tests/test_compatibility.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_compatibility.py b/tests/test_compatibility.py index 1925a91c96..00505352e7 100644 --- a/tests/test_compatibility.py +++ b/tests/test_compatibility.py @@ -8,7 +8,7 @@ import meshroom.core from meshroom.core import desc, registerNodeType, unregisterNodeType -from meshroom.core.exception import NodeUpgradeError +from meshroom.core.exception import GraphCompatibilityError, NodeUpgradeError from meshroom.core.graph import Graph, loadGraph from meshroom.core.node import CompatibilityNode, CompatibilityIssue, Node @@ -395,3 +395,24 @@ def test_conformUpgrade(): unregisterNodeType(SampleNodeV5) unregisterNodeType(SampleNodeV6) + + +class TestGraphLoadingWithStrictCompatibility: + + def test_failsOnNodeDescriptionCompatibilityIssue(self, graphSavedOnDisk): + registerNodeType(SampleNodeV1) + registerNodeType(SampleNodeV2) + + graph: Graph = graphSavedOnDisk + graph.addNewNode(SampleNodeV1.__name__) + graph.save() + + # Replace saved node description by V2 + meshroom.core.nodesDesc[SampleNodeV1.__name__] = SampleNodeV2 + + with pytest.raises(GraphCompatibilityError): + loadGraph(graph.filepath, strictCompatibility=True) + + unregisterNodeType(SampleNodeV1) + unregisterNodeType(SampleNodeV2) + From 98f1a9d516a731680ac46d31b014755d465f1694 Mon Sep 17 00:00:00 2001 From: Yann Lanthony Date: Thu, 14 Nov 2024 19:11:08 +0100 Subject: [PATCH 3/5] [tests] Update tests to load graphs with strictCompatibility Update tests that should fail if nodes are loaded as CompatibilityNodes (resulting in false positives). --- tests/test_nodeAttributeChangedCallback.py | 28 ++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/test_nodeAttributeChangedCallback.py b/tests/test_nodeAttributeChangedCallback.py index 42d73e1219..edd14bc8dc 100644 --- a/tests/test_nodeAttributeChangedCallback.py +++ b/tests/test_nodeAttributeChangedCallback.py @@ -156,7 +156,7 @@ def test_loadingGraphDoesNotTriggerCallback(self, graphSavedOnDisk): node.affectedInput.value = 2 graph.save() - loadedGraph = loadGraph(graph.filepath) + loadedGraph = loadGraph(graph.filepath, strictCompatibility=True) loadedNode = loadedGraph.node(node.name) assert loadedNode assert loadedNode.affectedInput.value == 2 @@ -170,11 +170,13 @@ def test_loadingGraphDoesNotTriggerCallbackForConnectedAttributes( graph.addEdge(nodeA.input, nodeB.input) nodeA.input.value = 5 + assert nodeB.affectedInput.value == nodeB.input.value * 2 + nodeB.affectedInput.value = 2 graph.save() - loadedGraph = loadGraph(graph.filepath) + loadedGraph = loadGraph(graph.filepath, strictCompatibility=True) loadedNodeB = loadedGraph.node(nodeB.name) assert loadedNodeB assert loadedNodeB.affectedInput.value == 2 @@ -407,3 +409,25 @@ def test_clearingDynamicOutputValueDoesNotTriggerDownstreamAttributeChangedCallb nodeA.clearData() assert nodeA.output.value == nodeB.input.value is None assert nodeB.affectedInput.value == expectedPreClearValue + + def test_loadingGraphWithComputedDynamicOutputValueDoesNotTriggerDownstreamAttributeChangedCallback( + self, graphSavedOnDisk + ): + graph: Graph = graphSavedOnDisk + nodeA = graph.addNewNode(NodeWithDynamicOutputValue.__name__) + nodeB = graph.addNewNode(NodeWithAttributeChangedCallback.__name__) + + nodeA.input.value = 10 + graph.addEdge(nodeA.output, nodeB.input) + executeGraph(graph) + + assert nodeA.output.value == nodeB.input.value == 20 + assert nodeB.affectedInput.value == 0 + + graph.save() + + loadGraph(graph.filepath, strictCompatibility=True) + + assert nodeB.affectedInput.value == 0 + + From 61d35904ba9e77914dc14ebfc1e850015c1569e3 Mon Sep 17 00:00:00 2001 From: Yann Lanthony Date: Thu, 14 Nov 2024 19:11:08 +0100 Subject: [PATCH 4/5] [core] Discard attribute callbacks during graph loading --- meshroom/core/graph.py | 13 +++++++++++++ meshroom/core/node.py | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index ebe2de1ba5..9fd07900c1 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -214,6 +214,7 @@ def getFeaturesForVersion(fileVersion): def __init__(self, name, parent=None): super(Graph, self).__init__(parent) self.name = name + self._loading = False self._updateEnabled = True self._updateRequested = False self.dirtyTopology = False @@ -246,6 +247,11 @@ def fileFeatures(self): """ Get loaded file supported features based on its version. """ return Graph.IO.getFeaturesForVersion(self.header.get(Graph.IO.Keys.FileVersion, "0.0")) + @property + def isLoading(self): + """ Return True if the graph is currently being loaded. """ + return self._loading + @Slot(str) def load(self, filepath, setupProjectFile=True, importProject=False, publishOutputs=False): """ @@ -259,6 +265,13 @@ def load(self, filepath, setupProjectFile=True, importProject=False, publishOutp of opened. publishOutputs: True if "Publish" nodes from templates should not be ignored. """ + self._loading = True + try: + self._load(filepath, setupProjectFile, importProject, publishOutputs) + finally: + self._loading = False + + def _load(self, filepath, setupProjectFile, importProject, publishOutputs): if not importProject: self.clear() with open(filepath) as jsonFile: diff --git a/meshroom/core/node.py b/meshroom/core/node.py index f6b4a85fe5..d206414d68 100644 --- a/meshroom/core/node.py +++ b/meshroom/core/node.py @@ -964,6 +964,10 @@ def _onAttributeChanged(self, attr: Attribute): if attr.value is None: # Discard dynamic values depending on the graph processing. return + + if self.graph and self.graph.isLoading: + # Do not trigger attribute callbacks during the graph loading. + return callback = self._getAttributeChangedCallback(attr) From b0808f904071fd260eb23af3f1bc60889298e8f9 Mon Sep 17 00:00:00 2001 From: Yann Lanthony Date: Mon, 18 Nov 2024 10:07:02 +0100 Subject: [PATCH 5/5] [core] Fix errorneous docstring Co-authored-by: Vivek --- meshroom/core/graph.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meshroom/core/graph.py b/meshroom/core/graph.py index 9fd07900c1..54f069fdd8 100644 --- a/meshroom/core/graph.py +++ b/meshroom/core/graph.py @@ -1658,7 +1658,7 @@ def loadGraph(filepath, strictCompatibility: bool = False) -> Graph: Graph: The loaded Graph instance. Raises: - GraphCompatibilityError: If the Graph has node compatibility issues and `strictCompatibility` is False. + GraphCompatibilityError: If the Graph has node compatibility issues and `strictCompatibility` is True. """ graph = Graph("") graph.load(filepath)