Skip to content

Commit 212b198

Browse files
committed
fix: rule modifiers, Python 3.12 compat, CI overhaul, NFsim version
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.
1 parent e3b7a2a commit 212b198

10 files changed

Lines changed: 208 additions & 17 deletions

File tree

.github/workflows/ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ jobs:
4848
shell: bash
4949
- name: Build
5050
run: |
51-
python setup.py install
52-
python setup.py sdist bdist_wheel
51+
python -m pip install --upgrade pip
52+
python -m pip install .
5353
- name: Test with PyTest
5454
run: |
5555
pytest

.github/workflows/release-test.yml

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: release-test
2+
3+
on:
4+
release:
5+
types: [published]
6+
7+
jobs:
8+
test-release:
9+
runs-on: ${{ matrix.os }}
10+
strategy:
11+
matrix:
12+
os: [ubuntu-latest, windows-latest, macos-latest]
13+
python-version: ["3.9", "3.10", "3.11", "3.12"]
14+
steps:
15+
- uses: actions/checkout@v4
16+
17+
- name: Set up Python ${{ matrix.python-version }}
18+
uses: actions/setup-python@v5
19+
with:
20+
python-version: ${{ matrix.python-version }}
21+
22+
- name: Install bionetgen from PyPI
23+
run: |
24+
python -m pip install --upgrade pip
25+
python -m pip install bionetgen
26+
27+
- name: Smoke test bionetgen command
28+
run: |
29+
bionetgen -h
30+
python -c "import bionetgen; print('bionetgen', bionetgen.__version__)"
31+
32+
- name: Run unit tests
33+
run: |
34+
python -m pip install pytest
35+
pytest

bionetgen/__main__.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
from .main import main
2+
3+
if __name__ == "__main__":
4+
main()

bionetgen/core/tools/info.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,50 @@ def gatherInfo(self):
6464
# Save version info
6565
self.info["Perl version"] = text[num_start:num_end] + " (used to run BNG2.pl)"
6666

67+
# Get NFsim version (if available on PATH or adjacent to BNG2.pl)
68+
self.logger.debug("NFsim info", loc=f"{__file__} : BNGInfo.gatherInfo()")
69+
nf_version_text = "not found"
70+
try:
71+
# Try the standard PATH lookup first
72+
result = subprocess.run(
73+
["NFsim", "--version"],
74+
stdout=subprocess.PIPE,
75+
stderr=subprocess.PIPE,
76+
text=True,
77+
timeout=10,
78+
)
79+
if result.returncode == 0:
80+
nf_version_text = result.stdout.strip().splitlines()[0]
81+
else:
82+
nf_version_text = f"exit {result.returncode}"
83+
except FileNotFoundError:
84+
# If NFsim isn't on PATH, attempt to locate it relative to BNG2.pl
85+
try:
86+
bng2_path = self.config.get("bionetgen", "bngpath")
87+
bng2_dir = os.path.dirname(bng2_path)
88+
candidates = [
89+
os.path.join(bng2_dir, "bin", "NFsim"),
90+
os.path.join(bng2_dir, "bin", "NFsim.exe"),
91+
]
92+
for cmd in candidates:
93+
if os.path.isfile(cmd):
94+
result = subprocess.run(
95+
[cmd, "--version"],
96+
stdout=subprocess.PIPE,
97+
stderr=subprocess.PIPE,
98+
text=True,
99+
timeout=10,
100+
)
101+
if result.returncode == 0:
102+
nf_version_text = result.stdout.strip().splitlines()[0]
103+
break
104+
except Exception:
105+
pass
106+
except Exception as e:
107+
nf_version_text = f"error: {e}"
108+
109+
self.info["NFsim version"] = nf_version_text
110+
67111
self.logger.debug("PyBNG info", loc=f"{__file__} : BNGInfo.gatherInfo()")
68112
# Get CLI version
69113
with open(

bionetgen/core/utils/utils.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import os, subprocess
1+
import os
2+
import shutil
3+
import subprocess
24
from bionetgen.core.exc import BNGPerlError
3-
from distutils import spawn
45

56
from bionetgen.core.utils.logging import BNGLogger
67

@@ -589,7 +590,7 @@ def _try_path(candidate_path):
589590
return hit
590591

591592
# 3) On PATH
592-
bng_on_path = spawn.find_executable("BNG2.pl")
593+
bng_on_path = shutil.which("BNG2.pl")
593594
if bng_on_path:
594595
tried.append(bng_on_path)
595596
hit = _try_path(bng_on_path)
@@ -616,7 +617,7 @@ def test_perl(app=None, perl_path=None):
616617
logger.debug("Checking if perl is installed.", loc=f"{__file__} : test_perl()")
617618
# find path to perl binary
618619
if perl_path is None:
619-
perl_path = spawn.find_executable("perl")
620+
perl_path = shutil.which("perl")
620621
if perl_path is None:
621622
raise BNGPerlError
622623
# check if perl is actually working

bionetgen/modelapi/xmlparsers.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -702,10 +702,11 @@ def get_rule_mod(self, xml):
702702
del_op = list_ops["Delete"]
703703
if not isinstance(del_op, list):
704704
del_op = [del_op] # Make sure del_op is list
705-
dmvals = [op["@DeleteMolecules"] for op in del_op]
706-
# All Delete operations in rule must have DeleteMolecules attribute or
707-
# it does not apply to the whole rule
708-
if all(dmvals) == 1:
705+
706+
# Use get() to avoid KeyError if the attribute is missing.
707+
dmvals = [op.get("@DeleteMolecules") for op in del_op]
708+
# All Delete operations in rule must have DeleteMolecules attribute set to "1".
709+
if all(dmvals) and all(str(v) == "1" for v in dmvals):
709710
rule_mod.type = "DeleteMolecules"
710711
# JRF: I don't believe the id of the specific op rule_mod is currently used
711712
# rule_mod.id = op["@id"]
@@ -731,21 +732,22 @@ def get_rule_mod(self, xml):
731732
for mo in move_op:
732733
if mo["@moveConnected"] == "1":
733734
rule_mod.type = "MoveConnected"
734-
rule_mod.id.append(move_op["@id"])
735-
rule_mod.source.append(move_op["@source"])
736-
rule_mod.destination.append(move_op["@destination"])
737-
rule_mod.flip.append(move_op["@flipOrientation"])
735+
rule_mod.id.append(mo["@id"])
736+
rule_mod.source.append(mo["@source"])
737+
rule_mod.destination.append(mo["@destination"])
738+
rule_mod.flip.append(mo["@flipOrientation"])
738739
rule_mod.call.append(mo["@moveConnected"])
739740
elif "RateLaw" in xml:
740741
# check if modifier is called
741742
ratelaw = xml["RateLaw"]
742743
rate_type = ratelaw["@type"]
743-
if rate_type == "Function" and ratelaw["@totalrate"] == 1:
744+
# @totalrate comes as a string in the XML
745+
if rate_type == "Function" and str(ratelaw.get("@totalrate")) == "1":
744746
rule_mod.type = "TotalRate"
745747
rule_mod.id = ratelaw["@id"]
746748
rule_mod.rate_type = ratelaw["@type"]
747749
rule_mod.name = ratelaw["@name"]
748-
rule_mod.call = ratelaw["@totalrate"]
750+
rule_mod.call = ratelaw.get("@totalrate")
749751

750752
# TODO: add support for include/exclude reactants/products
751753
if (

bionetgen/simulator/csimulator.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import ctypes, os, tempfile, bionetgen
22
import numpy as np
33

4-
from distutils import ccompiler
4+
# distutils is deprecated in Python 3.12+. setuptools still provides the
5+
# equivalent via setuptools._distutils for backwards compatibility.
6+
try:
7+
from setuptools._distutils import ccompiler
8+
except ImportError:
9+
from distutils import ccompiler
10+
511
from .bngsimulator import BNGSimulator
612
from bionetgen.main import BioNetGen
713
from bionetgen.core.exc import BNGCompileError

setup.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ def get_folder(arch):
186186
[console_scripts]
187187
bionetgen = bionetgen.main:main
188188
""",
189+
python_requires=">=3.8",
189190
install_requires=[
190191
"cement",
191192
"nbopen",
@@ -201,6 +202,9 @@ def get_folder(arch):
201202
"python-libsbml",
202203
"pylru",
203204
"pyparsing",
205+
"pyyed",
206+
"matplotlib",
207+
"pandas",
204208
"packaging",
205209
],
206210
)

tests/test_action_parsing.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import pytest
2+
3+
from bionetgen.core.utils.utils import ActionList
4+
5+
6+
def test_action_parser_rejects_unclosed_brace():
7+
"""Ensure malformed actions (missing closing brace) raise a parsing error."""
8+
9+
alist = ActionList()
10+
alist.define_parser()
11+
12+
# Missing closing '}' should cause pyparsing to raise an exception
13+
malformed = "simulate_ssa({t_start=>0,t_end=>10" # missing closing '}' and ')'
14+
15+
with pytest.raises(Exception):
16+
alist.action_parser.parseString(malformed)

tests/test_rule_modifiers.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import pytest
2+
3+
from bionetgen.modelapi.xmlparsers import RuleBlockXML
4+
5+
6+
def _rule_block_parser():
7+
# Create a RuleBlockXML instance without running __init__ (which expects full rule XML)
8+
return RuleBlockXML.__new__(RuleBlockXML)
9+
10+
11+
def test_get_rule_mod_total_rate_string_true():
12+
xml = {
13+
"ListOfOperations": {},
14+
"RateLaw": {"@type": "Function", "@totalrate": "1", "@id": "r1", "@name": "foo"},
15+
}
16+
17+
mod = _rule_block_parser().get_rule_mod(xml)
18+
assert mod.type == "TotalRate"
19+
assert mod.id == "r1"
20+
assert mod.call == "1"
21+
22+
23+
def test_get_rule_mod_delete_molecules_all_operations():
24+
xml = {
25+
"ListOfOperations": {
26+
"Delete": [
27+
{"@DeleteMolecules": "1"},
28+
{"@DeleteMolecules": "1"},
29+
]
30+
}
31+
}
32+
33+
mod = _rule_block_parser().get_rule_mod(xml)
34+
assert mod.type == "DeleteMolecules"
35+
36+
37+
def test_get_rule_mod_delete_molecules_missing_attribute_does_not_apply():
38+
xml = {
39+
"ListOfOperations": {
40+
"Delete": [
41+
{"@DeleteMolecules": "1"},
42+
{},
43+
]
44+
}
45+
}
46+
47+
mod = _rule_block_parser().get_rule_mod(xml)
48+
assert mod.type is None
49+
50+
51+
def test_get_rule_mod_move_connected_list_uses_each_element():
52+
xml = {
53+
"ListOfOperations": {
54+
"ChangeCompartment": [
55+
{
56+
"@moveConnected": "1",
57+
"@id": "a",
58+
"@source": "s",
59+
"@destination": "d",
60+
"@flipOrientation": "0",
61+
},
62+
{
63+
"@moveConnected": "1",
64+
"@id": "b",
65+
"@source": "s2",
66+
"@destination": "d2",
67+
"@flipOrientation": "1",
68+
},
69+
]
70+
}
71+
}
72+
73+
mod = _rule_block_parser().get_rule_mod(xml)
74+
assert mod.type == "MoveConnected"
75+
assert mod.id == ["a", "b"]
76+
assert mod.source == ["s", "s2"]
77+
assert mod.destination == ["d", "d2"]
78+
assert mod.flip == ["0", "1"]
79+
assert mod.call == ["1", "1"]

0 commit comments

Comments
 (0)