From 212b198cefbd60ec570fc7b82fb28c453718117e Mon Sep 17 00:00:00 2001 From: akutuva21 Date: Tue, 17 Mar 2026 16:02:34 -0400 Subject: [PATCH 1/3] fix: rule modifiers, Python 3.12 compat, CI overhaul, NFsim version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #55 — get_rule_mod() had three bugs in xmlparsers.py: - TotalRate never detected: string "1" compared to int 1 (always False) - DeleteMolecules: KeyError if @DeleteMolecules attribute absent on any op - MoveConnected list branch: appended from move_op[...] instead of mo[...] Fixes #57 — NFsim version now detected and reported in --version/info commands via PATH lookup with fallback to BNG2.pl-adjacent bin/. Fixes #58, #61 — Remove distutils usage throughout: - distutils.spawn → shutil.which in utils.py - distutils.ccompiler → setuptools._distutils with distutils fallback - setup.py: add python_requires>=3.8, promote missing deps to install_requires - ci.yml: replace deprecated setup.py install/sdist/bdist_wheel with pip Fixes #60 — Add release-test.yml workflow that installs from PyPI and runs smoke tests + pytest on every published release across all platforms. Fixes #21 — Add test_action_parsing.py covering pyparsing rejection of malformed actions (unclosed braces). Also adds bionetgen/__main__.py to support python -m bionetgen invocation, and tests/test_rule_modifiers.py with full coverage of the three get_rule_mod bug fixes. --- .github/workflows/ci.yml | 4 +- .github/workflows/release-test.yml | 35 +++++++++++++ bionetgen/__main__.py | 4 ++ bionetgen/core/tools/info.py | 44 +++++++++++++++++ bionetgen/core/utils/utils.py | 9 ++-- bionetgen/modelapi/xmlparsers.py | 22 +++++---- bionetgen/simulator/csimulator.py | 8 ++- setup.py | 4 ++ tests/test_action_parsing.py | 16 ++++++ tests/test_rule_modifiers.py | 79 ++++++++++++++++++++++++++++++ 10 files changed, 208 insertions(+), 17 deletions(-) create mode 100644 .github/workflows/release-test.yml create mode 100644 bionetgen/__main__.py create mode 100644 tests/test_action_parsing.py create mode 100644 tests/test_rule_modifiers.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c043652..c949084 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,8 +48,8 @@ jobs: shell: bash - name: Build run: | - python setup.py install - python setup.py sdist bdist_wheel + python -m pip install --upgrade pip + python -m pip install . - name: Test with PyTest run: | pytest diff --git a/.github/workflows/release-test.yml b/.github/workflows/release-test.yml new file mode 100644 index 0000000..c6a207f --- /dev/null +++ b/.github/workflows/release-test.yml @@ -0,0 +1,35 @@ +name: release-test + +on: + release: + types: [published] + +jobs: + test-release: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + python-version: ["3.9", "3.10", "3.11", "3.12"] + steps: + - uses: actions/checkout@v4 + + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + - name: Install bionetgen from PyPI + run: | + python -m pip install --upgrade pip + python -m pip install bionetgen + + - name: Smoke test bionetgen command + run: | + bionetgen -h + python -c "import bionetgen; print('bionetgen', bionetgen.__version__)" + + - name: Run unit tests + run: | + python -m pip install pytest + pytest diff --git a/bionetgen/__main__.py b/bionetgen/__main__.py new file mode 100644 index 0000000..40e2b01 --- /dev/null +++ b/bionetgen/__main__.py @@ -0,0 +1,4 @@ +from .main import main + +if __name__ == "__main__": + main() diff --git a/bionetgen/core/tools/info.py b/bionetgen/core/tools/info.py index dd07105..c50a1bf 100644 --- a/bionetgen/core/tools/info.py +++ b/bionetgen/core/tools/info.py @@ -64,6 +64,50 @@ def gatherInfo(self): # Save version info self.info["Perl version"] = text[num_start:num_end] + " (used to run BNG2.pl)" + # Get NFsim version (if available on PATH or adjacent to BNG2.pl) + self.logger.debug("NFsim info", loc=f"{__file__} : BNGInfo.gatherInfo()") + nf_version_text = "not found" + try: + # Try the standard PATH lookup first + result = subprocess.run( + ["NFsim", "--version"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + timeout=10, + ) + if result.returncode == 0: + nf_version_text = result.stdout.strip().splitlines()[0] + else: + nf_version_text = f"exit {result.returncode}" + except FileNotFoundError: + # If NFsim isn't on PATH, attempt to locate it relative to BNG2.pl + try: + bng2_path = self.config.get("bionetgen", "bngpath") + bng2_dir = os.path.dirname(bng2_path) + candidates = [ + os.path.join(bng2_dir, "bin", "NFsim"), + os.path.join(bng2_dir, "bin", "NFsim.exe"), + ] + for cmd in candidates: + if os.path.isfile(cmd): + result = subprocess.run( + [cmd, "--version"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + timeout=10, + ) + if result.returncode == 0: + nf_version_text = result.stdout.strip().splitlines()[0] + break + except Exception: + pass + except Exception as e: + nf_version_text = f"error: {e}" + + self.info["NFsim version"] = nf_version_text + self.logger.debug("PyBNG info", loc=f"{__file__} : BNGInfo.gatherInfo()") # Get CLI version with open( diff --git a/bionetgen/core/utils/utils.py b/bionetgen/core/utils/utils.py index 7d19fd2..3821b99 100644 --- a/bionetgen/core/utils/utils.py +++ b/bionetgen/core/utils/utils.py @@ -1,6 +1,7 @@ -import os, subprocess +import os +import shutil +import subprocess from bionetgen.core.exc import BNGPerlError -from distutils import spawn from bionetgen.core.utils.logging import BNGLogger @@ -589,7 +590,7 @@ def _try_path(candidate_path): return hit # 3) On PATH - bng_on_path = spawn.find_executable("BNG2.pl") + bng_on_path = shutil.which("BNG2.pl") if bng_on_path: tried.append(bng_on_path) hit = _try_path(bng_on_path) @@ -616,7 +617,7 @@ def test_perl(app=None, perl_path=None): logger.debug("Checking if perl is installed.", loc=f"{__file__} : test_perl()") # find path to perl binary if perl_path is None: - perl_path = spawn.find_executable("perl") + perl_path = shutil.which("perl") if perl_path is None: raise BNGPerlError # check if perl is actually working diff --git a/bionetgen/modelapi/xmlparsers.py b/bionetgen/modelapi/xmlparsers.py index 93d703c..0fea3c4 100644 --- a/bionetgen/modelapi/xmlparsers.py +++ b/bionetgen/modelapi/xmlparsers.py @@ -702,10 +702,11 @@ 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] - # All Delete operations in rule must have DeleteMolecules attribute or - # it does not apply to the whole rule - if all(dmvals) == 1: + + # Use get() to avoid KeyError if the attribute is missing. + dmvals = [op.get("@DeleteMolecules") for op in del_op] + # All Delete operations in rule must have DeleteMolecules attribute set to "1". + if all(dmvals) and all(str(v) == "1" for v 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 +732,22 @@ 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: + # @totalrate comes as a string in the XML + if rate_type == "Function" and str(ratelaw.get("@totalrate")) == "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") # TODO: add support for include/exclude reactants/products if ( diff --git a/bionetgen/simulator/csimulator.py b/bionetgen/simulator/csimulator.py index 34a6bdc..3818115 100644 --- a/bionetgen/simulator/csimulator.py +++ b/bionetgen/simulator/csimulator.py @@ -1,7 +1,13 @@ import ctypes, os, tempfile, bionetgen import numpy as np -from distutils import ccompiler +# distutils is deprecated in Python 3.12+. setuptools still provides the +# equivalent via setuptools._distutils for backwards compatibility. +try: + from setuptools._distutils import ccompiler +except ImportError: + from distutils import ccompiler + from .bngsimulator import BNGSimulator from bionetgen.main import BioNetGen from bionetgen.core.exc import BNGCompileError diff --git a/setup.py b/setup.py index 478d749..c712260 100644 --- a/setup.py +++ b/setup.py @@ -186,6 +186,7 @@ def get_folder(arch): [console_scripts] bionetgen = bionetgen.main:main """, + python_requires=">=3.8", install_requires=[ "cement", "nbopen", @@ -201,6 +202,9 @@ def get_folder(arch): "python-libsbml", "pylru", "pyparsing", + "pyyed", + "matplotlib", + "pandas", "packaging", ], ) diff --git a/tests/test_action_parsing.py b/tests/test_action_parsing.py new file mode 100644 index 0000000..f6e4a6d --- /dev/null +++ b/tests/test_action_parsing.py @@ -0,0 +1,16 @@ +import pytest + +from bionetgen.core.utils.utils import ActionList + + +def test_action_parser_rejects_unclosed_brace(): + """Ensure malformed actions (missing closing brace) raise a parsing error.""" + + alist = ActionList() + alist.define_parser() + + # Missing closing '}' should cause pyparsing to raise an exception + malformed = "simulate_ssa({t_start=>0,t_end=>10" # missing closing '}' and ')' + + with pytest.raises(Exception): + alist.action_parser.parseString(malformed) diff --git a/tests/test_rule_modifiers.py b/tests/test_rule_modifiers.py new file mode 100644 index 0000000..1ec5098 --- /dev/null +++ b/tests/test_rule_modifiers.py @@ -0,0 +1,79 @@ +import pytest + +from bionetgen.modelapi.xmlparsers import RuleBlockXML + + +def _rule_block_parser(): + # Create a RuleBlockXML instance without running __init__ (which expects full rule XML) + return RuleBlockXML.__new__(RuleBlockXML) + + +def test_get_rule_mod_total_rate_string_true(): + xml = { + "ListOfOperations": {}, + "RateLaw": {"@type": "Function", "@totalrate": "1", "@id": "r1", "@name": "foo"}, + } + + mod = _rule_block_parser().get_rule_mod(xml) + assert mod.type == "TotalRate" + assert mod.id == "r1" + assert mod.call == "1" + + +def test_get_rule_mod_delete_molecules_all_operations(): + xml = { + "ListOfOperations": { + "Delete": [ + {"@DeleteMolecules": "1"}, + {"@DeleteMolecules": "1"}, + ] + } + } + + mod = _rule_block_parser().get_rule_mod(xml) + assert mod.type == "DeleteMolecules" + + +def test_get_rule_mod_delete_molecules_missing_attribute_does_not_apply(): + xml = { + "ListOfOperations": { + "Delete": [ + {"@DeleteMolecules": "1"}, + {}, + ] + } + } + + mod = _rule_block_parser().get_rule_mod(xml) + assert mod.type is None + + +def test_get_rule_mod_move_connected_list_uses_each_element(): + xml = { + "ListOfOperations": { + "ChangeCompartment": [ + { + "@moveConnected": "1", + "@id": "a", + "@source": "s", + "@destination": "d", + "@flipOrientation": "0", + }, + { + "@moveConnected": "1", + "@id": "b", + "@source": "s2", + "@destination": "d2", + "@flipOrientation": "1", + }, + ] + } + } + + mod = _rule_block_parser().get_rule_mod(xml) + assert mod.type == "MoveConnected" + assert mod.id == ["a", "b"] + assert mod.source == ["s", "s2"] + assert mod.destination == ["d", "d2"] + assert mod.flip == ["0", "1"] + assert mod.call == ["1", "1"] From 9c99c744d70b4643b0db9b3c3663b973299de10f Mon Sep 17 00:00:00 2001 From: akutuva21 Date: Tue, 17 Mar 2026 16:06:09 -0400 Subject: [PATCH 2/3] Black lint fix --- tests/test_rule_modifiers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_rule_modifiers.py b/tests/test_rule_modifiers.py index 1ec5098..732da7f 100644 --- a/tests/test_rule_modifiers.py +++ b/tests/test_rule_modifiers.py @@ -11,7 +11,12 @@ def _rule_block_parser(): def test_get_rule_mod_total_rate_string_true(): xml = { "ListOfOperations": {}, - "RateLaw": {"@type": "Function", "@totalrate": "1", "@id": "r1", "@name": "foo"}, + "RateLaw": { + "@type": "Function", + "@totalrate": "1", + "@id": "r1", + "@name": "foo", + }, } mod = _rule_block_parser().get_rule_mod(xml) From 49dc1aca03a690796b67771c16c9ec1de3502a23 Mon Sep 17 00:00:00 2001 From: akutuva21 Date: Tue, 17 Mar 2026 16:25:50 -0400 Subject: [PATCH 3/3] fix: update setup.py to prevent pip install during import time --- setup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c712260..cffe2f9 100644 --- a/setup.py +++ b/setup.py @@ -14,7 +14,9 @@ def get_folder(arch): return fname -subprocess.check_call([sys.executable, "-m", "pip", "install", "numpy"]) +# Note: don't run pip install at import time; in PEP 517 builds pip isn't available +# and running subprocesses during import breaks isolated build environments. +# numpy is declared in install_requires and will be installed by pip. import urllib.request import itertools as itt