fix: keep everything of every model in merge_models; name the subsystem of transport reactions

.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

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_<sourceid>``). 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,11 +159,15 @@ 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:
                 if gene.id not in genes_before:
                     gene.notes.setdefault("origin", origin)
 
     merged._compartments.update(comp_names)
+    if objective:
+        merged.objective = objective
     return merged

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 <from> and <to>`` for a reversible reaction and
+``Transport from <from> to <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})
 

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

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"]