Merge pull request #67 from akutuva21/main

Bug Fixes for XML Parser in `get_rule_mod`

Author: jrfaeder

bionetgen/modelapi/xmlparsers.py

@@ -702,10 +702,10 @@ def get_rule_mod(self, xml):
             del_op = list_ops["Delete"]
             if not isinstance(del_op, list):
                 del_op = [del_op]  # Make sure del_op is list
-            dmvals = [op["@DeleteMolecules"] for op in del_op]
+            dmvals = [op.get("@DeleteMolecules", "0") for op in del_op]
             # All Delete operations in rule must have DeleteMolecules attribute or
             # it does not apply to the whole rule
-            if all(dmvals) == 1:
+            if all(val == "1" for val in dmvals):
                 rule_mod.type = "DeleteMolecules"
                 # JRF: I don't believe the id of the specific op rule_mod is currently used
                 # rule_mod.id = op["@id"]
@@ -731,21 +731,21 @@ def get_rule_mod(self, xml):
                 for mo in move_op:
                     if mo["@moveConnected"] == "1":
                         rule_mod.type = "MoveConnected"
-                        rule_mod.id.append(move_op["@id"])
-                        rule_mod.source.append(move_op["@source"])
-                        rule_mod.destination.append(move_op["@destination"])
-                        rule_mod.flip.append(move_op["@flipOrientation"])
+                        rule_mod.id.append(mo["@id"])
+                        rule_mod.source.append(mo["@source"])
+                        rule_mod.destination.append(mo["@destination"])
+                        rule_mod.flip.append(mo["@flipOrientation"])
                         rule_mod.call.append(mo["@moveConnected"])
         elif "RateLaw" in xml:
             # check if modifier is called
             ratelaw = xml["RateLaw"]
             rate_type = ratelaw["@type"]
-            if rate_type == "Function" and ratelaw["@totalrate"] == 1:
+            if rate_type == "Function" and str(ratelaw.get("@totalrate", "0")) == "1":
                 rule_mod.type = "TotalRate"
                 rule_mod.id = ratelaw["@id"]
                 rule_mod.rate_type = ratelaw["@type"]
                 rule_mod.name = ratelaw["@name"]
-                rule_mod.call = ratelaw["@totalrate"]
+                rule_mod.call = ratelaw.get("@totalrate", "0")
 
         # TODO: add support for include/exclude reactants/products
         if (

tests/test_get_rule_mod.py

@@ -0,0 +1,125 @@
+import pytest
+from bionetgen.modelapi.xmlparsers import RuleBlockXML
+
+
+def test_get_rule_mod():
+    parser = RuleBlockXML([])
+
+    xml_totalrate_int = {
+        "@name": "test_rule",
+        "ListOfOperations": {},
+        "RateLaw": {
+            "@type": "Function",
+            "@totalrate": 1,
+            "@id": "rule1",
+            "@name": "rate1",
+        },
+    }
+    mod1 = parser.get_rule_mod(xml_totalrate_int)
+    assert mod1.type == "TotalRate"
+    assert str(mod1.call) == "1"
+
+    xml_totalrate_str = {
+        "@name": "test_rule",
+        "ListOfOperations": {},
+        "RateLaw": {
+            "@type": "Function",
+            "@totalrate": "1",
+            "@id": "rule1",
+            "@name": "rate1",
+        },
+    }
+    mod1_str = parser.get_rule_mod(xml_totalrate_str)
+    assert mod1_str.type == "TotalRate"
+    assert mod1_str.call == "1"
+
+    xml_totalrate_missing = {
+        "@name": "test_rule",
+        "ListOfOperations": {},
+        "RateLaw": {"@type": "Function", "@id": "rule1", "@name": "rate1"},
+    }
+    mod1_miss = parser.get_rule_mod(xml_totalrate_missing)
+    assert mod1_miss.type is None
+
+    xml_totalrate_zero = {
+        "@name": "test_rule",
+        "ListOfOperations": {},
+        "RateLaw": {
+            "@type": "Function",
+            "@totalrate": "0",
+            "@id": "rule1",
+            "@name": "rate1",
+        },
+    }
+    mod1_zero = parser.get_rule_mod(xml_totalrate_zero)
+    assert mod1_zero.type is None
+
+    xml_delete = {
+        "@name": "test_rule",
+        "ListOfOperations": {
+            "Delete": [{"@DeleteMolecules": "1"}, {"@DeleteMolecules": "1"}]
+        },
+    }
+    mod2 = parser.get_rule_mod(xml_delete)
+    assert mod2.type == "DeleteMolecules"
+
+    xml_delete_missing = {"@name": "test_rule", "ListOfOperations": {"Delete": [{}]}}
+    mod3 = parser.get_rule_mod(xml_delete_missing)
+    assert mod3.type is None
+
+    xml_move = {
+        "@name": "test_rule",
+        "ListOfOperations": {
+            "ChangeCompartment": [
+                {
+                    "@moveConnected": "1",
+                    "@id": "m1",
+                    "@source": "s1",
+                    "@destination": "d1",
+                    "@flipOrientation": "0",
+                },
+                {
+                    "@moveConnected": "1",
+                    "@id": "m2",
+                    "@source": "s2",
+                    "@destination": "d2",
+                    "@flipOrientation": "1",
+                },
+            ]
+        },
+    }
+    mod4 = parser.get_rule_mod(xml_move)
+    assert mod4.type == "MoveConnected"
+    assert mod4.id == ["m1", "m2"]
+    assert mod4.source == ["s1", "s2"]
+
+    xml_move_single = {
+        "@name": "test_rule",
+        "ListOfOperations": {
+            "ChangeCompartment": {
+                "@moveConnected": "1",
+                "@id": "m1",
+                "@source": "s1",
+                "@destination": "d1",
+                "@flipOrientation": "0",
+            }
+        },
+    }
+    mod5 = parser.get_rule_mod(xml_move_single)
+    assert mod5.type == "MoveConnected"
+    assert mod5.id == "m1"
+    assert mod5.source == "s1"
+
+    # Precedence: Delete + RateLaw
+    xml_both = {
+        "@name": "test_rule",
+        "ListOfOperations": {"Delete": [{"@DeleteMolecules": "1"}]},
+        "RateLaw": {
+            "@type": "Function",
+            "@totalrate": "1",
+            "@id": "rule1",
+            "@name": "rate1",
+        },
+    }
+    mod6 = parser.get_rule_mod(xml_both)
+    assert mod6.type == "DeleteMolecules"