From ec19797eae8b5a117193708b75219b22af58aa7f Mon Sep 17 00:00:00 2001 From: Phil Date: Sat, 25 Jul 2026 21:37:52 -0400 Subject: [PATCH 1/2] fix: restore lineqlist2str, which CNApy imports at startup lineqlist2str was removed in 53f98ad as dead code. It had no caller in this repository or its tests, which is what I checked, and that was the wrong test: CNApy imports it in gui_elements/strain_design_dialog.py, and main_window.py imports that module at startup, so the removal turns CNApy into an ImportError on launch rather than a broken corner of one dialog. 1.19 is already published to PyPI and to the cnapy channel, and CNApy requires straindesign>=1.18 with cnapy as its highest-priority channel, so any fresh install resolves the broken version. This restores the function verbatim; it was a removal and not a rename, distinct from lineqlist2mat and from linexprdict2str, which it calls. Of the nine names 1.19 removed, this is the only one CNApy uses: modules_coeff2float, remove_redundant_bounds, lineq2mat, get_rids, the nullspace alias, compression_backend, compress_model_efmtool and suppressed_reactions are all unused there. CNApy's efmtool and jpype imports come from efmtool_link, its own dependency, and are unaffected. test_13_public_api.py pins the names CNApy imports. Grepping this repository cannot see a downstream consumer, so the check has to be written down rather than rediscovered. Also quote the version in conda-recipe/meta.yaml. Rendered unquoted, a version like 1.19 is YAML for a floating-point number; conda-build tolerates it but the conda-forge linter rejects it, and the two recipes should not disagree. Co-Authored-By: Claude Opus 4.8 (1M context) --- conda-recipe/meta.yaml | 2 +- straindesign/parse_constr.py | 22 +++++++++++++++++ tests/test_13_public_api.py | 48 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 tests/test_13_public_api.py diff --git a/conda-recipe/meta.yaml b/conda-recipe/meta.yaml index b8245de..8adbe6c 100644 --- a/conda-recipe/meta.yaml +++ b/conda-recipe/meta.yaml @@ -2,7 +2,7 @@ package: name: straindesign - version: {{ version }} + version: "{{ version }}" source: path: .. diff --git a/straindesign/parse_constr.py b/straindesign/parse_constr.py index 81843f7..e6fcc6d 100644 --- a/straindesign/parse_constr.py +++ b/straindesign/parse_constr.py @@ -125,6 +125,28 @@ def lineq2list(equations, reaction_ids) -> List: return D +def lineqlist2str(D): + """Translates a *linear* (in)equality from the list format [lhs,sign,rhs] to a string + + E.g. input: D=[{"a":3.0,"b":-1.0,"c":2.0},"<=",2.0]] is translated to: out="3.0 a - 1.0 b + 2.0 c <= 2" + + Args: + D (list): + (In)equality in list form, e.g.: D=[{"a":3.0,"b":-1.0,"c":2.0},"<=",2.0]] + + Returns: + (str): + A list of (in)equalities in string form + + """ + if D[0]: + return linexprdict2str(D[0]) + " " + D[1] + " " + str(D[2]) + elif D[1] and D[2]: + return D[1] + " " + str(D[2]) + else: + return "" + + def lineqlist2mat(D, reaction_ids) -> Tuple[sparse.csr_matrix, Tuple, sparse.csr_matrix, Tuple]: """Translates *linear* (in)equalities presented in the list of lists format to matrices diff --git a/tests/test_13_public_api.py b/tests/test_13_public_api.py new file mode 100644 index 0000000..3bf781c --- /dev/null +++ b/tests/test_13_public_api.py @@ -0,0 +1,48 @@ +"""Test that the public API downstream packages import stays importable. + +Being unused inside this repository is not evidence that a public name is unused: +`lineqlist2str` had no caller here and was deleted as dead code, but CNApy imports +it at startup in gui_elements/strain_design_dialog.py, so the removal broke the +application. These names are re-checked here because grepping this repository +cannot see that. +""" +import importlib + +import pytest + +# Imported by CNApy; see cnapy/gui_elements/strain_design_dialog.py and siblings. +CNAPY_IMPORTS = { + "straindesign": [ + "avail_solvers", + "compute_strain_designs", + "fba", + "lineq2list", + "lineqlist2str", + "linexpr2dict", + "linexprdict2str", + "plot_flux_space", + "SDModule", + "select_solver", + "yopt", + ], + "straindesign.parse_constr": ["lineq2list", "linexpr2dict", "linexprdict2str"], + "straindesign.names": ["CPLEX", "GLPK", "GUROBI", "SCIP"], + "straindesign.strainDesignSolutions": ["SDSolutions"], +} + + +@pytest.mark.parametrize("module,names", sorted(CNAPY_IMPORTS.items())) +def test_public_names_are_importable(module, names): + """Names that downstream packages import must remain importable.""" + mod = importlib.import_module(module) + missing = [n for n in names if not hasattr(mod, n)] + assert not missing, f"{module} no longer exports: {', '.join(missing)}" + + +def test_lineqlist2str_formats_an_inequality(): + """lineqlist2str renders [lhs, sign, rhs] as a string, as CNApy displays it.""" + from straindesign import lineqlist2str + + assert lineqlist2str([{"a": 3.0, "b": -1.0}, "<=", 2.0]) == "3.0 a - 1.0 b <= 2.0" + assert lineqlist2str([{}, "<=", 2.0]) == "<= 2.0" + assert lineqlist2str([{}, "", ""]) == "" From d637302e88c5b110c90cf1c60484132081e16fee Mon Sep 17 00:00:00 2001 From: Phil Date: Sat, 25 Jul 2026 21:47:58 -0400 Subject: [PATCH 2/2] test: check CNApy's imports against this package on the quarterly run The hardcoded list of names CNApy imports goes stale the moment CNApy imports something new, and nothing in this repository can notice: a downstream consumer is invisible to grep here. So read CNApy instead of installing it. The test fetches its source, walks the AST for every `from straindesign ... import`, and asserts each name is still exported. Installing CNApy was the obvious alternative and is a bad trade: it depends on Qt and PyQtWebEngine, on cplex, which this matrix already cannot install on macOS or 3.13, and on jpype, which is the JVM binding removed in #72 and the reason those tests were Windows-only. That would drag all of it back into twenty jobs to test one import list. It is marked `downstream` and skipped unless --downstream is passed, following the existing --medium and --large convention, and the workflow passes the flag only on the quarterly schedule. It needs network and tracks a branch we do not control, so it reports drift rather than gating pull requests. The static list stays. It runs everywhere, offline and unconditionally, and it is the layer that would have blocked the removal in the first place. Verified against ffad74b, before the restore: the AST check fails there, as do both static guards. A test that cannot fail is decoration. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/CI-test.yml | 5 +++- tests/conftest.py | 9 ++++-- tests/test_13_public_api.py | 56 +++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/.github/workflows/CI-test.yml b/.github/workflows/CI-test.yml index 6d6c010..aeff78d 100644 --- a/.github/workflows/CI-test.yml +++ b/.github/workflows/CI-test.yml @@ -131,5 +131,8 @@ jobs: if: matrix.package-manager == 'conda' run: | micromamba install pytest pytest-timeout + # --downstream reads CNApy's source to check the names it imports from this + # package. It needs network and tracks a branch we do not control, so it is + # limited to the quarterly scheduled run rather than gating pull requests. - name: Test with pytest - run: pytest tests -v --medium --log-cli-level=INFO + run: pytest tests -v --medium ${{ github.event_name == 'schedule' && '--downstream' || '' }} --log-cli-level=INFO diff --git a/tests/conftest.py b/tests/conftest.py index 23d3ff5..7096f05 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,7 +4,8 @@ from straindesign.names import * # --------------------------------------------------------------------------- -# Custom CLI flags for test_performance.py tiered benchmarks +# Custom CLI flags for opt-in tests: the tiered benchmarks in +# test_09_performance.py and the downstream check in test_13_public_api.py # --------------------------------------------------------------------------- @@ -12,6 +13,7 @@ def pytest_addoption(parser): for name, help_text in [ ("--medium", "Run iMLcore genome-scale benchmarks (~4 min total)."), ("--large", "Run iML1515 large-model benchmarks (several min/solver)."), + ("--downstream", "Check the names CNApy imports against this package (needs network)."), ]: try: parser.addoption(name, action="store_true", default=False, help=help_text) @@ -23,6 +25,7 @@ def pytest_configure(config): for marker, desc in [ ("medium", "genome-scale benchmark; enable with --medium"), ("large", "large-model benchmark; enable with --large"), + ("downstream", "downstream consumer check; enable with --downstream"), ]: config.addinivalue_line("markers", f"{marker}: {desc}") # Suppress known third-party warnings @@ -31,9 +34,9 @@ def pytest_configure(config): def pytest_collection_modifyitems(config, items): - for flag, marker in [("--medium", "medium"), ("--large", "large")]: + for flag, marker in [("--medium", "medium"), ("--large", "large"), ("--downstream", "downstream")]: if not config.getoption(flag, default=False): - skip = pytest.mark.skip(reason=f"pass {flag} to enable this benchmark") + skip = pytest.mark.skip(reason=f"pass {flag} to enable this test") for item in items: if marker in item.keywords: item.add_marker(skip) diff --git a/tests/test_13_public_api.py b/tests/test_13_public_api.py index 3bf781c..c083293 100644 --- a/tests/test_13_public_api.py +++ b/tests/test_13_public_api.py @@ -6,10 +6,17 @@ application. These names are re-checked here because grepping this repository cannot see that. """ +import ast import importlib +import io +import tarfile +import urllib.request +import warnings import pytest +CNAPY_TARBALL = "https://github.com/cnapy-org/CNApy/archive/refs/heads/master.tar.gz" + # Imported by CNApy; see cnapy/gui_elements/strain_design_dialog.py and siblings. CNAPY_IMPORTS = { "straindesign": [ @@ -39,6 +46,55 @@ def test_public_names_are_importable(module, names): assert not missing, f"{module} no longer exports: {', '.join(missing)}" +def _straindesign_names_imported_by(source): + """Yield (module, name) for every straindesign import in a source file.""" + with warnings.catch_warnings(): + # Parsing someone else's source can raise SyntaxWarning for things like + # invalid escape sequences. Those are CNApy's to fix, not signal here. + warnings.simplefilter("ignore", SyntaxWarning) + tree = ast.parse(source) + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + if node.level == 0 and node.module and node.module.split(".")[0] == "straindesign": + for alias in node.names: + if alias.name != "*": + yield node.module, alias.name + + +@pytest.mark.downstream +def test_cnapy_master_imports_still_exist(): + """Every straindesign name CNApy imports must still be exported. + + CNApy is read rather than installed: it depends on Qt, a JVM via jpype, and + CPLEX, none of which belong in this matrix. Reading the source catches names + CNApy has newly imported, which the hardcoded list above cannot. + """ + try: + with urllib.request.urlopen(CNAPY_TARBALL, timeout=120) as response: + payload = response.read() + except Exception as exc: # offline, rate-limited, or the branch moved + pytest.skip(f"could not fetch CNApy source: {exc}") + + wanted = set() + with tarfile.open(fileobj=io.BytesIO(payload), mode="r:gz") as tar: + for member in tar.getmembers(): + if not member.name.endswith(".py"): + continue + handle = tar.extractfile(member) + if handle is None: + continue + wanted.update(_straindesign_names_imported_by(handle.read().decode("utf-8", "replace"))) + + assert wanted, "found no straindesign imports in CNApy; the parser or the URL is wrong" + + missing = [] + for module, name in sorted(wanted): + mod = importlib.import_module(module) + if not hasattr(mod, name): + missing.append(f"{module}.{name}") + assert not missing, ("CNApy imports names this package no longer exports: " + ", ".join(missing)) + + def test_lineqlist2str_formats_an_inequality(): """lineqlist2str renders [lhs, sign, rhs] as a string, as CNApy displays it.""" from straindesign import lineqlist2str