diff --git a/src/diagram_connectivity.py b/src/diagram_connectivity.py index f3399f8..d331696 100644 --- a/src/diagram_connectivity.py +++ b/src/diagram_connectivity.py @@ -20,7 +20,7 @@ import os from collections import defaultdict from pathlib import Path -from typing import Set, Tuple +from typing import Dict, List, Set, Tuple import pandas as pd @@ -162,3 +162,63 @@ def augment_reaction_connections(pathway_id: str, f"pairs not in precedingEvent (of {len(pairs)} drawn)" ) return pd.concat([reaction_connections, pd.DataFrame(new_rows)], ignore_index=True) + + +def diagram_glyph_positions(pathway_id: str) -> Dict[Tuple[str, str, str], List[int]]: + """``(reaction_stId, entity_stId, role) -> [glyph_id]`` for one pathway. + + Adam: *"two nodes in the reactome pathway diagram that are in the same + compartment could be the same thing but in two different places. We need + to be able to know which one the uuid was from."* + + That identity exists and this module already read it — ``x["id"]`` on each + input/output/catalyst entry is the DiagramObject id, which is what the + pathway browser selects and highlights — and then discarded it after + pairing producers with consumers. This exposes it instead. + + The triple is a **unique** key, which is what makes the question + answerable: measured on R-HSA-1257604, 0 of 156 triples resolve to more + than one glyph. An entity drawn nine times (ATP, ADP) is drawn once per + reaction, so naming the reaction and the role disambiguates it exactly. + The return type is still a list because a future diagram could break that + assumption, and silently returning the first of several would hide it. + + Returns an empty mapping when no diagram covers the pathway. + """ + ddir = _diagram_dir() + diagram_stid = _covering_diagram_stid(pathway_id) + if not diagram_stid: + return {} + + layout = json.loads((ddir / f"{diagram_stid}.json").read_text()) + graph = json.loads((ddir / f"{diagram_stid}.graph.json").read_text()) + + edge_dbid_to_stid = {e["dbId"]: e["stId"] for e in graph.get("edges", []) if e.get("stId")} + node_dbid_to_stid = {n["dbId"]: n["stId"] for n in graph.get("nodes", []) if n.get("stId")} + glyph_to_entity_dbid = {n["id"]: n.get("reactomeId") for n in layout.get("nodes", [])} + + roles = {"inputs": "input", "outputs": "output", "catalysts": "catalyst"} + positions: Dict[Tuple[str, str, str], List[int]] = {} + for edge in layout.get("edges", []): + reaction_stid = edge_dbid_to_stid.get(edge.get("reactomeId")) + if not reaction_stid: + continue + for key, role in roles.items(): + for entry in edge.get(key, []): + glyph_id = entry.get("id") + entity_stid = node_dbid_to_stid.get(glyph_to_entity_dbid.get(glyph_id)) + if glyph_id is None or not entity_stid: + continue + positions.setdefault((reaction_stid, entity_stid, role), []) + if glyph_id not in positions[(reaction_stid, entity_stid, role)]: + positions[(reaction_stid, entity_stid, role)].append(glyph_id) + return positions + + +def covering_diagram_stid(pathway_id: str) -> str: + """The diagram a pathway is drawn in — its own, or the nearest ancestor's. + + A glyph id is unique only WITHIN a diagram, so it is meaningless without + this. Exposed so callers can record the pair together. + """ + return _covering_diagram_stid(pathway_id) diff --git a/src/logic_network_generator.py b/src/logic_network_generator.py index 162304c..b49cde1 100755 --- a/src/logic_network_generator.py +++ b/src/logic_network_generator.py @@ -2470,6 +2470,8 @@ def export_node_resolution(pathway_id: str, get_reactome_release, get_set_members, get_modifier_isoform_entity_set_ids) from src.set_resolution import make_neo4j_resolver + from src.diagram_connectivity import (covering_diagram_stid, + diagram_glyph_positions) release = get_reactome_release() release_str = str(release) if release is not None else "" @@ -2477,6 +2479,15 @@ def export_node_resolution(pathway_id: str, vr_to_reaction = dict(zip(reaction_id_map["uid"].astype(str), reaction_id_map["reactome_id"].astype(str))) + # (reaction, entity, role) -> glyph ids. A glyph id is unique only within + # its diagram, so the diagram is recorded with it or neither is written. + try: + glyph_positions = diagram_glyph_positions(pathway_id) + diagram_stid = covering_diagram_stid(pathway_id) or "" + except Exception: + logger.warning(f"no diagram glyph positions for {pathway_id}") + glyph_positions, diagram_stid = {}, "" + rows: List[Dict[str, Any]] = [] seen: Set[tuple] = set() @@ -2488,12 +2499,18 @@ def add(stable_id: str, node_uuid: str, relation: str, depth: int, if key in seen: return seen.add(key) + # The diagram draws an entity once PER REACTION, so naming the + # reaction and the role identifies the glyph exactly — measured, 0 of + # 156 triples in R-HSA-1257604 resolve to more than one, while four + # entities are drawn up to nine times. That is what makes "which of + # the two ATP glyphs did this uuid come from" answerable. + glyphs = glyph_positions.get((reaction_stid, stable_id, role), []) + glyph_id = str(glyphs[0]) if len(glyphs) == 1 else "" rows.append({ "stable_id": stable_id, "uuid": node_uuid, "relation": relation, "depth": depth, "role": role, "reaction_stid": reaction_stid, - # Populated by the diagram work (US3); the columns exist now so - # consumers do not have to branch on schema version later. - "glyph_id": "", "diagram_stid": "", + "glyph_id": glyph_id, + "diagram_stid": diagram_stid if glyph_id else "", "release": release_str, }) diff --git a/tests/test_provenance_exports.py b/tests/test_provenance_exports.py index 4092994..8148d8c 100644 --- a/tests/test_provenance_exports.py +++ b/tests/test_provenance_exports.py @@ -146,3 +146,44 @@ def test_context_export_emits_no_orphaned_rows(tmp_path): "catalyst row must name the decomposed member the network wires up, " f"not the parent fetch-row UUID; got {catalysts[0]['context_node']}" ) + + +def test_glyph_id_and_diagram_are_written_together(tmp_path, monkeypatch): + """A glyph id without its diagram is meaningless, and vice versa. + + Glyph ids are unique only WITHIN a diagram — the same integer identifies a + different drawing in another one — so a row carrying one without the other + cannot be resolved back to anything. This is the invariant behind Adam's + question: knowing a uuid came from glyph 535 is only useful if you also + know which diagram 535 belongs to. + """ + monkeypatch.setattr(m, "get_labels", lambda e: ["EntityWithAccessionedSequence"], raising=False) + + rxn = "aaaaaaaa-0000-0000-0000-0000000000r1" + src = "aaaaaaaa-0000-0000-0000-000000000001" + edges = pd.DataFrame([ + {"source_id": src, "target_id": rxn, "pos_neg": "pos", "and_or": "and", + "edge_type": "input", "stoichiometry": 1, "edge_reaction_id": "R-HSA-100"}, + ]) + reaction_id_map = pd.DataFrame({"uid": [rxn], "reactome_id": ["R-HSA-100"]}) + + import src.diagram_connectivity as dc + # One entity at one reaction, drawn once: the triple resolves to a glyph. + monkeypatch.setattr(dc, "diagram_glyph_positions", + lambda pid: {("R-HSA-100", "R-HSA-999", "input"): [535]}) + monkeypatch.setattr(dc, "covering_diagram_stid", lambda pid: "R-HSA-1257604") + monkeypatch.setattr(m, "get_pathway_participating_entities", lambda pid: set(), raising=False) + + out = tmp_path / "node_resolution.csv" + exc = tmp_path / "node_exclusions.csv" + m.export_node_resolution("R-HSA-100", edges, reaction_id_map, + {src: "R-HSA-999"}, str(out), str(exc)) + + rows = pd.read_csv(out, dtype=str, keep_default_na=False).to_dict("records") + for row in rows: + has_glyph = bool(row["glyph_id"].strip()) + has_diagram = bool(row["diagram_stid"].strip()) + assert has_glyph == has_diagram, ( + f"glyph_id={row['glyph_id']!r} and diagram_stid={row['diagram_stid']!r} " + "must be present together or absent together" + )