Merge branch 'main' into modelapi-centralize-action-validation

Author: jrfaeder

bionetgen/core/utils/utils.py

@@ -105,6 +105,7 @@ def __init__(self):
             "max_agg",
             "max_iter",
             "max_stoich",
+            "check_iso",
             "TextReaction",
             "TextSpecies",
         ]
@@ -578,7 +579,10 @@ def define_parser(self):
         arg_type_list = "[" + pp.delimitedList((quote_word ^ arg_type_float)) + "]"
         arg_type_string = quote_word
         #
-        curly_arg_token = quote_word + "=>" + arg_type_int
+        # BNGL/Perl `=>` auto-quotes its left operand, so dict keys
+        # may be either bareword (max_stoich=>{R=>6}) or quoted
+        # (max_stoich=>{"R"=>6}). Accept both.
+        curly_arg_token = (base_name ^ quote_word) + "=>" + arg_type_int
         # TODO: handle 0 case
         arg_type_curly = "{" + pp.delimitedList(curly_arg_token) + "}"
         arg_types = (
@@ -746,20 +750,18 @@ def run_command(command, suppress=True, timeout=None, cwd=None):
     be killed.
     """
     if timeout is not None:
-        if suppress:
-            # I am unsure how to do both timeout and the live polling of stdo
-            rc = subprocess.run(
-                command,
-                timeout=timeout,
-                stdout=subprocess.DEVNULL,
-                stderr=subprocess.DEVNULL,
-                cwd=cwd,
-            )
-            return rc.returncode, rc
-        else:
-            # I am unsure how to do both timeout and the live polling of stdo
-            rc = subprocess.run(command, timeout=timeout, capture_output=True, cwd=cwd)
-            return rc.returncode, rc
+        # Always capture stdout/stderr — this lets callers (notably BNGCLI)
+        # surface BNG2.pl's error tail in BNGRunError when the command
+        # fails. With timeout set, subprocess.run buffers all output
+        # anyway, so suppress=True vs False makes no behavioral difference
+        # for the user during the run.
+        rc = subprocess.run(
+            command,
+            timeout=timeout,
+            capture_output=True,
+            cwd=cwd,
+        )
+        return rc.returncode, rc
     else:
         if suppress:
             process = subprocess.Popen(

bionetgen/modelapi/blocks.py

@@ -645,6 +645,30 @@ def gen_string(self) -> str:
         return "\n".join(block_lines)
 
 
+class ProtocolBlock(ActionBlock):
+    """
+    Protocol block object, subclass of ActionBlock.
+
+    Protocol lines live inside ``begin model``/``end model`` and must
+    retain their own begin/end block wrapper rather than being rendered
+    as top-level actions.
+    """
+
+    def __init__(self) -> None:
+        super().__init__()
+        self.name = "protocol"
+
+    def add_item(self, item_tpl) -> None:
+        _, value = item_tpl
+        self.items.append(value)
+
+    def gen_string(self) -> str:
+        block_lines = ["\nbegin protocol"]
+        block_lines.extend(item.print_line() for item in self.items)
+        block_lines.append("end protocol\n")
+        return "\n".join(block_lines)
+
+
 class EnergyPatternBlock(ModelBlock):
     """
     Energy pattern block object, subclass of ModelBlock.

bionetgen/modelapi/bngfile.py

@@ -54,6 +54,11 @@ def __init__(
         self.BNGPATH = BNGPATH
         self.bngexec = bngexec
         self.parsed_actions = []
+        # Actions that live inside a ``begin protocol``/``end protocol``
+        # block, stored separately from top-level actions so BNGParser can
+        # round-trip them into a ProtocolBlock instead of folding them into
+        # the top-level ActionBlock.
+        self.parsed_protocol_actions = []
 
     def generate_xml(self, xml_file, model_file=None) -> bool:
         """
@@ -150,17 +155,51 @@ def strip_actions(self, model_path, folder) -> str:
         with open(model_path, "r", encoding="UTF-8") as mf:
             # read and strip actions
             mstr = mf.read()
-            # TODO: Clean this up _a lot_
-            # this removes any new line escapes (\ \n) to continue
-            # to another line, so we can just remove the action lines
-            mstr = re.sub(r"\\\n", "", mstr)
+            # Collapse `\<newline>` line continuations before stripping
+            # action lines, so the action parser sees the same logical
+            # command boundaries as BNG.
+            #
+            # Only collapse `\` that appears before any `#` on its line.
+            # A continuation marker after the comment introducer is part
+            # of the comment body in BNG2.pl — collapsing it would glue
+            # the next physical line (often a real definition) into the
+            # comment, dropping it from the model. Repro: a commented-out
+            # `# foo()=if(t<42,0,\` immediately above a live
+            # `foo()=if(t<42,9.899,\` definition would silently lose the
+            # live function from the regenerated `.bngl` (and from any
+            # `.net` BNG2.pl generated downstream).
+            mstr = re.sub(r"^([^#\n]*)\\\n", r"\1", mstr, flags=re.MULTILINE)
             mlines = mstr.split("\n")
-            stripped_lines = list(filter(lambda x: self._not_action(x), mlines))
-            # remove spaces, actions don't allow them
-            self.parsed_actions = [
-                x.replace(" ", "")
-                for x in filter(lambda x: not self._not_action(x), mlines)
-            ]
+            # Walk the lines once, separating non-action content (kept in the
+            # stripped output for BNG2.pl) from action-shaped lines, and
+            # further splitting action-shaped lines based on whether they sit
+            # inside a ``begin protocol``/``end protocol`` block. Protocol
+            # actions are tracked separately so BNGParser can round-trip them
+            # into a ProtocolBlock instead of the top-level ActionBlock.
+            self.parsed_actions = []
+            self.parsed_protocol_actions = []
+            stripped_lines = []
+            in_protocol = False
+            for line in mlines:
+                if re.match(r"\s*(begin)\s+(protocol)\b", line):
+                    in_protocol = True
+                    stripped_lines.append(line)
+                    continue
+                if re.match(r"\s*(end)\s+(protocol)\b", line):
+                    in_protocol = False
+                    stripped_lines.append(line)
+                    continue
+                if self._not_action(line):
+                    stripped_lines.append(line)
+                    continue
+                # Hand the action line off to BNGParser intact — quoted
+                # spans (e.g. ``param=>"-v -gml 1000000"``) need to survive
+                # the whitespace-collapse pass, which `_normalize_action_text`
+                # does in a quote-aware way.
+                if in_protocol:
+                    self.parsed_protocol_actions.append(line)
+                else:
+                    self.parsed_actions.append(line)
             # let's remove begin/end actions, rarely used but should be removed
             remove_from = -1
             remove_to = -1
@@ -192,8 +231,14 @@ def strip_actions(self, model_path, folder) -> str:
         return stripped_model
 
     def _not_action(self, line) -> bool:
+        # Anchor the match to the start of the (left-stripped) line so that
+        # user identifiers containing an action name as a substring — most
+        # commonly ``conversion()`` (the substring ``version(`` matches the
+        # ``version`` action) inside a ``begin functions`` block — aren't
+        # misclassified and pulled out as actions.
+        stripped = line.lstrip()
         for action in self._action_list:
-            if action in line:
+            if stripped.startswith(action):
                 return False
         return True