diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8082da..d949202 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: python-version: "3.12" cache: pip cache-dependency-path: pyproject.toml - - run: pip install --upgrade pip + - run: python -m pip install --upgrade pip # Pinned so the lint result is reproducible (unpinned ruff silently # adds rules over time). Keep in sync with the ``dev`` extra in pyproject. - run: pip install ruff==0.15.15 @@ -38,7 +38,7 @@ jobs: python-version: "3.12" cache: pip cache-dependency-path: pyproject.toml - - run: pip install --upgrade pip + - run: python -m pip install --upgrade pip # Installs runtime deps + the dev extra (mypy). Third-party stubs are # absent for cobra/optlang/scipy; ignore_missing_imports handles that. - run: pip install -e ".[dev]" @@ -65,7 +65,7 @@ jobs: python-version: ${{ matrix.python }} cache: pip cache-dependency-path: pyproject.toml - - run: pip install --upgrade pip + - run: python -m pip install --upgrade pip # ``-e .[dev,excel]`` so every optional extra is exercised. # Gurobi is not installable on free runners; the relevant tests # skip themselves when ``optlang.gurobi_interface`` cannot import. @@ -98,7 +98,7 @@ jobs: python-version: "3.12" cache: pip cache-dependency-path: pyproject.toml - - run: pip install --upgrade pip + - run: python -m pip install --upgrade pip # gurobipy ships a size-limited licence (2000 variables / 2000 # constraints). That is useless at genome scale but ample for the small # fixtures here, and it gives the cross-solver check a second solver to diff --git a/src/raven_toolbox/manipulation/merge.py b/src/raven_toolbox/manipulation/merge.py index 175842b..8366562 100644 --- a/src/raven_toolbox/manipulation/merge.py +++ b/src/raven_toolbox/manipulation/merge.py @@ -5,7 +5,10 @@ compound under different ids in two models becomes one), while adding **all** reactions without de-duplication (a reaction whose ID already exists is renamed ``id_``). Genes are -unified by ID. Provenance (which source model each object came from) is recorded +unified by ID. Like RAVEN, every metabolite and gene of every model is kept, also +when no reaction uses it; the objective of every model is carried over; when two +models name the same compartment differently the first model's name is kept; new +metabolites and genes are added in the order of their source model. Provenance (which source model each object came from) is recorded in ``notes['origin']``. The bulk of RAVEN's function is struct field-padding and manual S-matrix @@ -18,7 +21,7 @@ from collections.abc import Iterable import cobra -from cobra import Metabolite, Model, Reaction +from cobra import Gene, Metabolite, Model, Reaction def _unique_id(existing, base: str, suffix: str) -> str: @@ -69,7 +72,9 @@ def merge_models( return models[0].copy() merged = Model("MERGED") + merged.objective_direction = models[0].objective_direction comp_names: dict[str, str] = {} + objective: dict[Reaction, float] = {} met_lookup: dict = {} # name/comp or id key -> merged Metabolite def met_key(met: Metabolite): @@ -118,9 +123,23 @@ def ensure_metabolite(src: Metabolite, origin: str) -> Metabolite: for model in models: origin = model.id or "model" - comp_names.update(model.compartments) + # model.compartments lists only compartments that hold metabolites; RAVEN keeps all. + for comp, name in {**model.compartments, **model._compartments}.items(): + comp_names.setdefault(comp, name) # the first model's name for a compartment wins genes_before = {g.id for g in merged.genes} + # Every metabolite and gene of the model, in the model's order, before the reactions + # that use them: unused ones are kept, and the order does not depend on the reactions. + for met in model.metabolites: + ensure_metabolite(met, origin) + for gene in model.genes: + if gene.id not in merged.genes: + new_gene = Gene(gene.id, name=gene.name) + new_gene.annotation = copy.deepcopy(gene.annotation) + new_gene.notes = copy.deepcopy(gene.notes) + merged.genes.append(new_gene) + new_gene._model = merged + new_rxns: list[Reaction] = [] for rxn in model.reactions: new_id = _unique_id(merged.reactions, rxn.id, origin) @@ -140,6 +159,8 @@ def ensure_metabolite(src: Metabolite, origin: str) -> Metabolite: new_rxn.notes = copy.deepcopy(rxn.notes) if track_origin: new_rxn.notes.setdefault("origin", origin) + if rxn.objective_coefficient: + objective[new_rxn] = rxn.objective_coefficient if track_origin: for gene in merged.genes: @@ -147,4 +168,6 @@ def ensure_metabolite(src: Metabolite, origin: str) -> Metabolite: gene.notes.setdefault("origin", origin) merged._compartments.update(comp_names) + if objective: + merged.objective = objective return merged diff --git a/src/raven_toolbox/manipulation/transport.py b/src/raven_toolbox/manipulation/transport.py index e3c717e..a544cbc 100644 --- a/src/raven_toolbox/manipulation/transport.py +++ b/src/raven_toolbox/manipulation/transport.py @@ -4,7 +4,10 @@ species by *name* across compartments (the source in ``from_compartment`` and its same-named twin in each target compartment), optionally creating the target metabolite, and -builds a ``-1 from / +1 to`` reaction with a sequential ``tr_0001`` ID. +builds a ``-1 from / +1 to`` reaction with a sequential ``tr_0001`` ID. As in RAVEN's +``addTransport``, the subsystem of each reaction names the two compartments: +``Transport between and `` for a reversible reaction and +``Transport from to `` for an irreversible one (the compartment ids are used). """ from __future__ import annotations @@ -159,6 +162,11 @@ def add_transport_reactions( rxn = Reaction(next_id()) rxn.name = f"{name} transport, {from_name}-{to_name}" rxn.bounds = bounds + rxn.subsystem = ( + f"Transport between {from_compartment} and {to_comp}" + if reversible + else f"Transport from {from_compartment} to {to_comp}" + ) added.append(rxn) pending_coeffs.append({src: -1, dst: 1}) diff --git a/tests/test_manipulation_merge.py b/tests/test_manipulation_merge.py index b789c96..e4ce91c 100644 --- a/tests/test_manipulation_merge.py +++ b/tests/test_manipulation_merge.py @@ -134,3 +134,125 @@ def test_charge_conflict_warns(): [{"id": "EX_B", "equation": "g2 -->"}]) with pytest.warns(UserWarning, match="different charges"): merge_models([a, b]) + + +# --- RAVEN's mergeModels keeps everything of every model ------------------------------ + +def _with_extras(): + """Model with a metabolite, a gene and a compartment that no reaction uses.""" + m = _model( + "X", + [cobra.Metabolite("a_c", name="A", compartment="c"), + cobra.Metabolite("b_c", name="B", compartment="c")], + [{"id": "R1", "equation": "a_c --> b_c", "gene_reaction_rule": "G1"}], + ) + m.add_metabolites([cobra.Metabolite("orph_c", name="Orphan", compartment="c")]) + orphan = cobra.Gene("G_ORPH", name="unused gene") + m.genes.append(orphan) + orphan._model = m + return m + + +def test_unused_metabolites_and_genes_are_kept(model_b): + merged = merge_models([_with_extras(), model_b]) + assert "orph_c" in merged.metabolites + assert "G_ORPH" in merged.genes + assert merged.genes.get_by_id("G_ORPH").name == "unused gene" + assert merged.genes.get_by_id("G_ORPH").notes["origin"] == "X" + + +def test_unused_items_of_a_later_model_are_kept_too(model_a): + merged = merge_models([model_a, _with_extras()]) + assert "orph_c" in merged.metabolites + assert "G_ORPH" in merged.genes + + +def test_objective_of_every_model_is_carried_over(model_a, model_b): + model_a.objective = "HEX" + model_b.objective = {model_b.reactions.get_by_id("LDH"): 2.0} + merged = merge_models([model_a, model_b]) + coefficients = {r.id: r.objective_coefficient for r in merged.reactions if r.objective_coefficient} + assert coefficients == {"HEX": 1.0, "LDH": 2.0} + + +def test_objective_follows_a_renamed_reaction(model_a): + other = _model( + "B", + [cobra.Metabolite("glc_c", name="Glucose", compartment="c"), + cobra.Metabolite("x_c", name="X", compartment="c")], + [{"id": "HEX", "equation": "glc_c --> x_c"}], + ) + other.objective = "HEX" + merged = merge_models([model_a, other]) + assert merged.reactions.get_by_id("HEX_B").objective_coefficient == 1.0 + assert merged.reactions.get_by_id("HEX").objective_coefficient == 0 + + +def test_no_objective_stays_empty(model_a, model_b): + merged = merge_models([model_a, model_b]) + assert not any(r.objective_coefficient for r in merged.reactions) + + +def test_objective_direction_comes_from_the_first_model(model_a, model_b): + model_a.objective = "HEX" + model_a.objective_direction = "min" + merged = merge_models([model_a, model_b]) + assert merged.objective_direction == "min" + + +def test_first_model_names_a_compartment(): + first = _model("A", [cobra.Metabolite("a_c", name="A", compartment="c")], []) + first.compartments = {"c": "cytoplasm"} + second = _model("B", [cobra.Metabolite("b_c", name="B", compartment="c")], []) + second.compartments = {"c": "cytosol", "e": "extracellular"} + merged = merge_models([first, second]) + assert merged.compartments["c"] == "cytoplasm" + assert merged._compartments["e"] == "extracellular" # a compartment without metabolites is kept too + + +def test_order_follows_the_source_models(model_a, model_b): + # model_b lists lac_c, then glucose_c: the merged order is model_a's metabolites, + # then the new ones of model_b in model_b's order (RAVEN appends in model order). + merged = merge_models([model_a, model_b]) + assert [m.name for m in merged.metabolites] == ["Glucose", "G6P", "Lactate"] + assert [g.id for g in merged.genes] == ["GA", "GB"] + + +def test_order_does_not_depend_on_the_reactions(): + # The reaction uses the second metabolite first; the model order is still kept. + m = cobra.Model("O") + m.add_metabolites([cobra.Metabolite("p_c", name="P", compartment="c"), + cobra.Metabolite("q_c", name="Q", compartment="c")]) + add_reactions_from_equations(m, [{"id": "R", "equation": "q_c --> p_c", "gene_reaction_rule": "G2 or G1"}]) + merged = merge_models([m, m.copy()]) + assert [x.id for x in merged.metabolites][:2] == ["p_c", "q_c"] + + +def test_merge_is_independent_of_the_hash_seed(): + import os + import subprocess + import sys + code = ( + "import cobra\n" + "from raven_toolbox.manipulation import merge_models\n" + "ms = []\n" + "for i in range(2):\n" + " m = cobra.Model(f'M{i}')\n" + " m.add_metabolites([cobra.Metabolite(f'm{j}_c', name=f'N{j}', compartment='c') for j in range(6)])\n" + " for k in (5, 3, 4, 1, 2, 0):\n" + " g = cobra.Gene(f'G{k}'); m.genes.append(g); g._model = m\n" + " r = cobra.Reaction(f'R{i}'); m.add_reactions([r])\n" + " r.add_metabolites({m.metabolites[0]: -1, m.metabolites[1]: 1})\n" + " r.gene_reaction_rule = ' or '.join(f'G{k}' for k in (5, 3, 4, 1, 2, 0))\n" + " ms.append(m)\n" + "g = merge_models(ms)\n" + "print([x.id for x in g.metabolites], [x.id for x in g.genes])\n" + ) + outputs = set() + for seed in ("0", "1", "2"): + # The seed is set explicitly (not "-I", which ignores PYTHON* variables), and the rest + # of the environment is kept: Python does not start on Windows without SYSTEMROOT. + out = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True, + env={**os.environ, "PYTHONHASHSEED": seed}, check=True).stdout + outputs.add(out) + assert len(outputs) == 1 diff --git a/tests/test_manipulation_transport.py b/tests/test_manipulation_transport.py index 6a5b83b..d4cdaaf 100644 --- a/tests/test_manipulation_transport.py +++ b/tests/test_manipulation_transport.py @@ -96,3 +96,20 @@ def test_duplicate_name_in_source_compartment_warns(model): added = add_transport_reactions(model, "c", "m", ["H2O"], only_to_existing=False) # Transport still works (uses the first match) — the warning is the signal. assert len(added) == 1 + + +# --- the subsystem names the two compartments, as in RAVEN's addTransport ----------------- + +def test_reversible_transport_subsystem(model): + added = add_transport_reactions(model, "c", "m", ["ATP"]) + assert added[0].subsystem == "Transport between c and m" + + +def test_irreversible_transport_subsystem(model): + added = add_transport_reactions(model, "c", "m", ["ATP"], reversible=False) + assert added[0].subsystem == "Transport from c to m" + + +def test_subsystem_names_each_target_compartment(model): + added = add_transport_reactions(model, "c", ["m", "e"], ["ATP"], only_to_existing=False) + assert [r.subsystem for r in added] == ["Transport between c and m", "Transport between c and e"]