Unify gdiff GraphML and traversal error handling

bionetgen/core/tools/gdiff.py

@@ -1,6 +1,7 @@
 from multiprocessing.sharedctypes import Value
 import xmltodict, copy, os, json
 
+from bionetgen.core.exc import BNGFileError
 from bionetgen.core.utils.logging import BNGLogger
 
 
@@ -84,6 +85,15 @@ def __init__(
         with open(self.input2, "r") as f:
             self.gdict_2 = xmltodict.parse(f.read())
 
+    def _graphml_file_error(self, message) -> BNGFileError:
+        return BNGFileError(getattr(self, "input", None), message=message)
+
+    def _describe_node(self, node) -> str:
+        node_id = self._get_node_id(node)
+        if node_id is None:
+            return "GraphML node"
+        return f"GraphML node {node_id}"
+
     def diff_graphs(
         self,
         g1,
@@ -493,6 +503,11 @@ def _get_node_from_names(self, g, names):
         return node
 
     def _get_node_properties(self, node):
+        node_desc = self._describe_node(node)
+        if "data" not in node:
+            raise self._graphml_file_error(
+                f"Could not find supported yEd properties for {node_desc}"
+            )
         if isinstance(node["data"], list):
             found = False
             for datum in node["data"]:
@@ -511,7 +526,9 @@ def _get_node_properties(self, node):
                         properties = snode
                     found = True
             if not found:
-                raise RuntimeError("Can't find properties for nodes")
+                raise self._graphml_file_error(
+                    f"Could not find supported yEd properties for {node_desc}"
+                )
         else:
             if "y:ProxyAutoBoundsNode" in node["data"].keys():
                 properties = node["data"]["y:ProxyAutoBoundsNode"]["y:Realizers"][
@@ -520,7 +537,9 @@ def _get_node_properties(self, node):
             elif "y:ShapeNode" in node["data"].keys():
                 properties = node["data"]["y:ShapeNode"]
             else:
-                raise RuntimeError("Can't find properties for nodes")
+                raise self._graphml_file_error(
+                    f"Could not find supported yEd properties for {node_desc}"
+                )
         return properties
 
     def _get_node_name(self, node):
@@ -559,7 +578,10 @@ def _get_color_id(self, node):
             # yellow indicates a state
             cid = 2
         else:
-            raise RuntimeError(f"Node color {curr_color} doesn't match known colors")
+            node_desc = self._describe_node(node)
+            raise self._graphml_file_error(
+                f"{node_desc} color {curr_color} doesn't match known BioNetGen contact-map colors"
+            )
         return cid
 
     def _get_node_from_keylist(self, g, keylist):
@@ -569,29 +591,34 @@ def _get_node_from_keylist(self, g, keylist):
             # we only have "graphml" as key
             return g[gkey]
         # we are out of group nodes
-        if "graph" not in g[gkey].keys():
+        graph = g[gkey].get("graph")
+        if not isinstance(graph, dict) or "node" not in graph:
             return None
         # everything up to here is good,
         # loop over to find the node
-        nodes = g[gkey]["graph"]["node"]
+        nodes = graph["node"]
+        node = None
         while len(copy_keylist) > 0:
             key = copy_keylist.pop(0)
-            found = False
             if isinstance(nodes, list):
                 for cnode in nodes:
-                    if cnode["@id"] == key:
-                        found = True
+                    if cnode.get("@id") == key:
                         node = cnode
-                        try:
-                            nodes = node["graph"]["node"]
-                        except:
-                            break
+                        break
+                else:
+                    node = None
+            elif isinstance(nodes, dict) and nodes.get("@id") == key:
+                node = nodes
             else:
-                if cnode["@id"] == key:
-                    found = True
-                    node = cnode
-            if not found:
+                node = None
+            if node is None:
+                return None
+            if len(copy_keylist) == 0:
+                return node
+            graph = node.get("graph")
+            if not isinstance(graph, dict) or "node" not in graph:
                 return None
+            nodes = graph["node"]
         return node
 
     def _color_node(self, node, color) -> bool:
@@ -605,15 +632,26 @@ def _color_node(self, node, color) -> bool:
                 color dictionary with g1/g2/intersect keys and color hex strings as values
         returns
             bool
-                True if colored correctly, False if not
+                True if colored correctly
+
+        raises
+            BNGFileError
+                if the GraphML node does not expose the expected yEd properties
         """
         try:
             fill = self._get_node_fill(node)
             fill["@color"] = color
             return True
+        except BNGFileError as exc:
+            self.logger.error(
+                f"Couldn't color {self._describe_node(node)}: {exc.message}",
+                loc=f"{__file__} : BNGGdiff._color_node()",
+            )
+            raise
         except Exception as e:
-            print(f"Couldn't color node, error: {e}")
-            return False
+            msg = f"Couldn't color {self._describe_node(node)}: {e}"
+            self.logger.error(msg, loc=f"{__file__} : BNGGdiff._color_node()")
+            raise self._graphml_file_error(msg) from e
 
     def _get_node_text(self, node):
         noded = node["data"]["y:ProxyAutoBoundsNode"]["y:Realizers"]

tests/test_gdiff.py

@@ -0,0 +1,195 @@
+import copy
+import json
+from unittest import mock
+
+import pytest
+import xmltodict
+
+from bionetgen.core.exc import BNGFileError
+from bionetgen.core.tools.gdiff import BNGGdiff
+
+
+def _make_shape_node(name, color, node_id, font_size="12"):
+    return {
+        "@id": node_id,
+        "data": {
+            "@key": "d6",
+            "y:ShapeNode": {
+                "y:Geometry": {"@height": "30", "@width": "30"},
+                "y:Fill": {"@color": color, "@transparent": "false"},
+                "y:NodeLabel": {"#text": name, "@fontSize": font_size},
+            },
+        },
+    }
+
+
+def _make_group_node(name, color, node_id, children, font_size="12"):
+    child_nodes = children if len(children) != 1 else children[0]
+    return {
+        "@id": node_id,
+        "data": [
+            {"@key": "d4"},
+            {
+                "@key": "d6",
+                "y:ProxyAutoBoundsNode": {
+                    "y:Realizers": {
+                        "y:GroupNode": {
+                            "y:Geometry": {"@height": "80", "@width": "120"},
+                            "y:Fill": {"@color": color, "@transparent": "false"},
+                            "y:NodeLabel": {"#text": name, "@fontSize": font_size},
+                        }
+                    }
+                },
+            },
+        ],
+        "graph": {"@id": node_id + ":", "node": child_nodes},
+    }
+
+
+def _make_edge(edge_id, source, target):
+    return {
+        "@id": edge_id,
+        "@source": source,
+        "@target": target,
+        "data": {"@key": "d10"},
+    }
+
+
+def _make_graphml(nodes, edges):
+    return {
+        "graphml": {
+            "@xmlns": "http://graphml.graphstruct.org/graphml",
+            "graph": {
+                "@id": "G",
+                "@edgedefault": "undirected",
+                "node": nodes,
+                "edge": edges,
+            },
+        }
+    }
+
+
+def _write_graphml(path, graph):
+    with open(path, "w") as handle:
+        xmltodict.unparse(graph, output=handle, pretty=True)
+
+
+def _read_graphml(path):
+    with open(path, "r") as handle:
+        return xmltodict.parse(handle.read(), force_list=("node", "edge"))
+
+
+GRAPH1 = _make_graphml(
+    [
+        _make_group_node(
+            "A",
+            "#D2D2D2",
+            "n0",
+            [
+                _make_shape_node("a1", "#FFFFFF", "n0::n0"),
+                _make_shape_node("a2", "#FFFFFF", "n0::n1"),
+            ],
+        ),
+        _make_group_node(
+            "B",
+            "#D2D2D2",
+            "n1",
+            [_make_shape_node("b1", "#FFFFFF", "n1::n0")],
+        ),
+    ],
+    [
+        _make_edge("e0", "n0::n0", "n1::n0"),
+        _make_edge("e1", "n0::n1", "n1::n0"),
+    ],
+)
+
+GRAPH2 = _make_graphml(
+    [
+        _make_group_node(
+            "A",
+            "#D2D2D2",
+            "n0",
+            [_make_shape_node("a1", "#FFFFFF", "n0::n0")],
+        ),
+        _make_group_node(
+            "C",
+            "#D2D2D2",
+            "n1",
+            [_make_shape_node("c1", "#FFFFFF", "n1::n0")],
+        ),
+    ],
+    [
+        _make_edge("e0", "n0::n0", "n1::n0"),
+        _make_edge("e1", "n1::n0", "n0::n0"),
+    ],
+)
+
+
+def _make_gdiff(path1, path2):
+    obj = BNGGdiff.__new__(BNGGdiff)
+    from bionetgen.core.utils.logging import BNGLogger
+
+    obj.app = None
+    obj.logger = BNGLogger(app=None)
+    obj.input = path1
+    obj.input2 = path2
+    obj.output = None
+    obj.output2 = None
+    obj.colors = {
+        "g1": ["#dadbfd", "#e6e7fe", "#f3f3ff"],
+        "g2": ["#ff9e81", "#ffbfaa", "#ffdfd4"],
+        "intersect": ["#c4ed9e", "#d9f4be", "#ecf9df"],
+    }
+    obj.available_modes = ["matrix", "union"]
+    obj.mode = "matrix"
+    obj.gdict_1 = _read_graphml(path1)
+    obj.gdict_2 = _read_graphml(path2)
+    return obj
+
+
+@pytest.fixture
+def gdiff_obj(tmp_path):
+    path1 = tmp_path / "g1.graphml"
+    path2 = tmp_path / "g2.graphml"
+    _write_graphml(path1, copy.deepcopy(GRAPH1))
+    _write_graphml(path2, copy.deepcopy(GRAPH2))
+    return _make_gdiff(str(path1), str(path2))
+
+
+def test_get_color_id_unknown_raises_bng_file_error(gdiff_obj):
+    node = _make_shape_node("x", "#123456", "n0")
+    with pytest.raises(
+        BNGFileError, match="doesn't match known BioNetGen contact-map colors"
+    ):
+        gdiff_obj._get_color_id(node)
+
+
+def test_get_node_properties_shape_without_supported_node_type_raises(gdiff_obj):
+    node = {"@id": "n0", "data": {"@key": "d6", "y:UnsupportedNode": {}}}
+    with pytest.raises(BNGFileError, match="Could not find supported yEd properties"):
+        gdiff_obj._get_node_properties(node)
+
+
+def test_color_node_logs_and_raises_for_invalid_node(gdiff_obj):
+    node = {"@id": "n0", "data": {"@key": "d6", "y:UnsupportedNode": {}}}
+    with mock.patch.object(gdiff_obj.logger, "error") as mock_error:
+        with pytest.raises(
+            BNGFileError, match="Could not find supported yEd properties"
+        ):
+            gdiff_obj._color_node(node, "#AABBCC")
+    mock_error.assert_called_once()
+    assert "Couldn't color GraphML node n0" in mock_error.call_args.args[0]
+
+
+def test_keylist_finds_nested_leaf_node(gdiff_obj):
+    graph = copy.deepcopy(gdiff_obj.gdict_1)
+    result = gdiff_obj._get_node_from_keylist(graph, ["graphml", "n0", "n0::n0"])
+    assert result["@id"] == "n0::n0"
+    assert gdiff_obj._get_node_name(result) == "a1"
+
+
+def test_keylist_finds_leaf_in_single_dict_child_graph(gdiff_obj):
+    graph = copy.deepcopy(gdiff_obj.gdict_1)
+    result = gdiff_obj._get_node_from_keylist(graph, ["graphml", "n1", "n1::n0"])
+    assert result["@id"] == "n1::n0"
+    assert gdiff_obj._get_node_name(result) == "b1"