From 5aec6fbde57434a9143580ba70254ca3d7874699 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Wed, 15 Jul 2026 22:36:44 +0200 Subject: [PATCH 01/11] ci: overhaul model-QC comment and result-file handling - Show "Model file and metabolic tasks" rows as running until the checks phase completes, instead of displaying the previous run's committed values. - Add QC check: flag reactions/metabolites removed since the base branch that were not moved to the deprecated identifier lists (qc_deprecation_completeness.csv). - Merge the structural-checks and model-QC-reports tables into one; link every check name to its explanation in the testResults README. - Reorganise data/testResults/README.md: per-file provenance, per-test explanations (anchors matching the comment), and a file index. - Combine the one-line result files (round-trip, YAML lint, metabolic tasks, growth) into a single qc_status.tsv via a qcStatus.py upsert helper. - Update the PR comment only after results are committed, in two phases (fast checks, then MEMOTE), so shown numbers and CSV links are always on the branch. - Store MEMOTE core-subset and full-suite scores in separate sections of memote_score.md so a routine run never overwrites a full-suite score; each section is compared only against the same section on the base branch. - Bump actions/github-script to v9 (Node 24) to clear the Node 20 deprecation. - Fix gene-essentiality README PR-number stamping (guarded on a never-true condition). --- .github/actions/post-qc-comment/action.yml | 11 +- .github/workflows/add-contributor.yml | 4 +- .github/workflows/gene-essentiality.yml | 8 +- .github/workflows/memote-full.yml | 7 +- .github/workflows/model-qc.yml | 126 ++++++--- code/test/buildReport.py | 153 ++++++++--- code/test/memoteSnapshot.py | 53 +++- code/test/qcModelChecks.py | 67 ++++- code/test/qcStatus.py | 72 +++++ code/test/testMetabolicTasks.py | 9 +- data/testResults/README.md | 255 ++++++++++++++---- data/testResults/memote_score.md | 6 + .../qc_deprecation_completeness.csv | 1 + data/testResults/qc_growth.txt | 1 - data/testResults/qc_roundtrip_cobra.txt | 1 - data/testResults/qc_roundtrip_raven.txt | 1 - data/testResults/qc_status.tsv | 7 + data/testResults/qc_tasks_essential.txt | 1 - data/testResults/qc_tasks_verification.txt | 1 - data/testResults/qc_yamllint.txt | 1 - 20 files changed, 627 insertions(+), 158 deletions(-) create mode 100644 code/test/qcStatus.py create mode 100644 data/testResults/qc_deprecation_completeness.csv delete mode 100644 data/testResults/qc_growth.txt delete mode 100644 data/testResults/qc_roundtrip_cobra.txt delete mode 100644 data/testResults/qc_roundtrip_raven.txt create mode 100644 data/testResults/qc_status.tsv delete mode 100644 data/testResults/qc_tasks_essential.txt delete mode 100644 data/testResults/qc_tasks_verification.txt delete mode 100644 data/testResults/qc_yamllint.txt diff --git a/.github/actions/post-qc-comment/action.yml b/.github/actions/post-qc-comment/action.yml index 55cee6b2..fae13a5d 100644 --- a/.github/actions/post-qc-comment/action.yml +++ b/.github/actions/post-qc-comment/action.yml @@ -2,6 +2,13 @@ name: Post QC comment description: Build the model-quality report and create or update the single QC comment on the pull request. inputs: + mode: + description: >- + "both" (default) builds the report and posts the comment; "build" only renders + data/testResults/model_qc_summary.md (so it can be committed first); "post" only + posts the already-rendered summary (use after the commit). + required: false + default: both running-groups: description: Groups still running (their rows show as running); "all", "memote", or empty. required: false @@ -32,6 +39,7 @@ runs: using: composite steps: - name: Build report + if: inputs.mode == 'both' || inputs.mode == 'build' shell: bash env: RUNNING_GROUPS: ${{ inputs.running-groups }} @@ -41,7 +49,8 @@ runs: run: python code/test/buildReport.py - name: Create or update the comment - uses: actions/github-script@v7 + if: inputs.mode == 'both' || inputs.mode == 'post' + uses: actions/github-script@v9 env: ISSUE_NUMBER: ${{ inputs.issue-number }} with: diff --git a/.github/workflows/add-contributor.yml b/.github/workflows/add-contributor.yml index be256866..68310790 100644 --- a/.github/workflows/add-contributor.yml +++ b/.github/workflows/add-contributor.yml @@ -29,7 +29,7 @@ jobs: steps: - name: Parse the command id: parse - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: script: | const body = context.payload.comment.body || ''; @@ -88,7 +88,7 @@ jobs: - name: Reply with the pull request link if: steps.parse.outputs.found == 'true' && steps.cpr.outputs.pull-request-number - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: script: | const n = '${{ steps.cpr.outputs.pull-request-number }}'; diff --git a/.github/workflows/gene-essentiality.yml b/.github/workflows/gene-essentiality.yml index a4a8d880..be78f01e 100644 --- a/.github/workflows/gene-essentiality.yml +++ b/.github/workflows/gene-essentiality.yml @@ -77,10 +77,12 @@ jobs: echo "GENEESS_EOF" } >> "$GITHUB_OUTPUT" + # This workflow is dispatched (by /run gene-essentiality), so it has no + # pull_request context; the PR number arrives as the `pr` input instead. - name: Mention PR# in README.md - if: github.event_name == 'pull_request' + if: inputs.pr != '' env: - PR_NUMBER: ${{ github.event.number }} + PR_NUMBER: ${{ inputs.pr }} run: sed -i -e "s/[[:digit:]]\{3,4\}\*\* (gene /$PR_NUMBER\*\* (gene /" data/testResults/README.md - name: Update local branch before committing changes @@ -110,7 +112,7 @@ jobs: - name: Post comment if: inputs.pr != '' - uses: actions/github-script@v7 + uses: actions/github-script@v9 env: TEST_RESULTS: ${{ steps.essentiality.outputs.results }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} diff --git a/.github/workflows/memote-full.yml b/.github/workflows/memote-full.yml index f7e7f099..80dc565d 100644 --- a/.github/workflows/memote-full.yml +++ b/.github/workflows/memote-full.yml @@ -15,10 +15,9 @@ env: RESULT_FILES: >- qc_duplicate_keys.csv qc_empty_reactions.csv qc_annotation_consistency.csv qc_unused_entities.csv qc_duplicate_reactions.csv qc_metabolite_completeness.csv - qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md - macaw_results.csv balance_results.csv qc_structure_consistency.csv - qc_roundtrip_cobra.txt qc_roundtrip_raven.txt qc_yamllint.txt - qc_tasks_essential.txt qc_tasks_verification.txt + qc_reaction_sanity.csv qc_annotation_issues.csv qc_deprecation_completeness.csv + qc_status.tsv memote_score.md macaw_results.csv balance_results.csv + qc_structure_consistency.csv jobs: memote-full: diff --git a/.github/workflows/model-qc.yml b/.github/workflows/model-qc.yml index 5b326f04..30b37e53 100644 --- a/.github/workflows/model-qc.yml +++ b/.github/workflows/model-qc.yml @@ -4,19 +4,23 @@ on: [pull_request] env: # Committed result files, fetched from the target branch so buildReport can show a - # delta. No stamp files: freshness is passed to buildReport as RUNNING_GROUPS. + # delta. No stamp files: freshness is passed to buildReport as RUNNING_GROUPS. The + # one-line checks (round-trip, yamllint, metabolic tasks) and the growth value all + # live together in qc_status.tsv (see qcStatus.py) rather than one file each. RESULT_FILES: >- qc_duplicate_keys.csv qc_empty_reactions.csv qc_annotation_consistency.csv qc_unused_entities.csv qc_duplicate_reactions.csv qc_metabolite_completeness.csv - qc_reaction_sanity.csv qc_annotation_issues.csv qc_growth.txt memote_score.md - macaw_results.csv balance_results.csv qc_structure_consistency.csv - qc_roundtrip_cobra.txt qc_roundtrip_raven.txt qc_yamllint.txt - qc_tasks_essential.txt qc_tasks_verification.txt - -# One job runs every check and edits a single pull-request comment as results come -# in: it posts a "running" comment immediately, fills in the fast checks, then fills -# in the MEMOTE score when it finishes. Results are committed once, at the end, and -# only if they changed - so a change that does not affect the model adds no commit. + qc_reaction_sanity.csv qc_annotation_issues.csv qc_deprecation_completeness.csv + qc_status.tsv memote_score.md macaw_results.csv balance_results.csv + qc_structure_consistency.csv + +# One job runs every check and edits a single pull-request comment. It posts a +# "running" comment immediately (all rows as hourglasses, no numbers), then works in +# two phases: it commits the fast-check results and updates the comment from the +# committed files (MEMOTE still a running row), then commits the MEMOTE score and +# updates the comment again. Each commit lands before its comment update, so the +# comment never reports numbers, or links to CSVs, that are not yet on the branch. +# Commits use [skip ci] and only happen if something changed. jobs: qc: runs-on: ubuntu-latest @@ -36,7 +40,9 @@ jobs: uses: actions/checkout@v7 - name: Configure - run: echo "BASE_DIR=$RUNNER_TEMP/base" >> "$GITHUB_ENV" + run: | + echo "BASE_DIR=$RUNNER_TEMP/base" >> "$GITHUB_ENV" + echo "BASE_MODEL_DIR=$RUNNER_TEMP/base-model" >> "$GITHUB_ENV" - name: Set up Python 3 uses: actions/setup-python@v6 @@ -53,6 +59,12 @@ jobs: for f in $RESULT_FILES; do git show "origin/$BASE_REF:data/testResults/$f" > "$BASE_DIR/$f" 2>/dev/null || rm -f "$BASE_DIR/$f" done + # Base-branch model tables: qcModelChecks.py diffs them against this model + # to flag identifiers removed here but not moved to a deprecated list. + mkdir -p "$BASE_MODEL_DIR" + for f in reactions.tsv metabolites.tsv; do + git show "origin/$BASE_REF:model/$f" > "$BASE_MODEL_DIR/$f" 2>/dev/null || rm -f "$BASE_MODEL_DIR/$f" + done # Immediate feedback: everything shows as running. - name: Post running comment @@ -95,11 +107,15 @@ jobs: - name: YAML round-trip (cobrapy) continue-on-error: true - run: python code/test/testYamlConversion.py --tool cobra && echo pass > data/testResults/qc_roundtrip_cobra.txt || echo fail > data/testResults/qc_roundtrip_cobra.txt + run: | + if python code/test/testYamlConversion.py --tool cobra; then r=pass; else r=fail; fi + python code/test/qcStatus.py roundtrip_cobra "$r" - name: YAML round-trip (RAVEN) continue-on-error: true - run: python code/test/testYamlConversion.py --tool raven-toolbox && echo pass > data/testResults/qc_roundtrip_raven.txt || echo fail > data/testResults/qc_roundtrip_raven.txt + run: | + if python code/test/testYamlConversion.py --tool raven-toolbox; then r=pass; else r=fail; fi + python code/test/qcStatus.py roundtrip_raven "$r" - name: YAML lint id: yamllint @@ -111,16 +127,19 @@ jobs: - name: Record YAML lint result if: always() - run: echo "${{ steps.yamllint.outcome == 'success' && 'pass' || 'fail' }}" > data/testResults/qc_yamllint.txt + run: python code/test/qcStatus.py yamllint "${{ steps.yamllint.outcome == 'success' && 'pass' || 'fail' }}" - name: Metabolic tasks (essential and verification) continue-on-error: true run: python code/test/testMetabolicTasks.py all - # Fast checks are in; MEMOTE is still running. - - name: Update comment with fast checks + # Fast checks are in; MEMOTE is still running. Commit the fast results first, + # then update the comment from the committed files - so every number shown is + # already on the branch. MEMOTE stays a running row until its own commit below. + - name: Render fast-check report uses: ./.github/actions/post-qc-comment with: + mode: build running-groups: memote base-ref: ${{ env.BASE_REF }} base-dir: ${{ env.BASE_DIR }} @@ -128,6 +147,43 @@ jobs: run-url: ${{ env.RUN_URL }} github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Mention PR# in README.md + env: + PR_NUMBER: ${{ github.event.number }} + run: | + for tag in "model QC" "MEMOTE" "MACAW"; do + sed -i -e "s/[[:digit:]]\{3,4\}\*\* ($tag/$PR_NUMBER\*\* ($tag/" data/testResults/README.md + done + + - name: Update local branch before committing (fast checks) + env: + BRANCH_NAME: ${{ github.head_ref || github.ref_name }} + run: | + git stash + git fetch + git checkout $BRANCH_NAME + if git stash list | grep -q 'stash@{'; then + git stash pop + fi + + - name: Commit fast-check results + uses: stefanzweifel/git-auto-commit-action@v7 + with: + commit_user_name: memote-bot + # [skip ci] so this results commit does not re-trigger the workflow. + commit_message: "chore: update model QC results (fast checks) [skip ci]" + file_pattern: data/testResults/* + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Update comment with fast checks + uses: ./.github/actions/post-qc-comment + with: + mode: post + run-url: ${{ env.RUN_URL }} + base-ref: ${{ env.BASE_REF }} + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Install MEMOTE dependencies run: pip install memote gurobipy @@ -155,10 +211,13 @@ jobs: timeout 2400 python code/test/memoteSnapshot.py \ || echo "::warning::MEMOTE did not finish within 2400s; score unavailable this run." - # Everything is in. - - name: Update comment with all results + # MEMOTE is in: render the final summary so it is part of the commit below, then + # post from the committed file. As with the fast phase, the comment is updated + # only after the results are committed (so its numbers and CSV links resolve). + - name: Render final report uses: ./.github/actions/post-qc-comment with: + mode: build running-groups: "" base-ref: ${{ env.BASE_REF }} base-dir: ${{ env.BASE_DIR }} @@ -166,15 +225,7 @@ jobs: run-url: ${{ env.RUN_URL }} github-token: ${{ secrets.GITHUB_TOKEN }} - - name: Mention PR# in README.md - env: - PR_NUMBER: ${{ github.event.number }} - run: | - for tag in "model QC" "MEMOTE" "MACAW"; do - sed -i -e "s/[[:digit:]]\{3,4\}\*\* ($tag/$PR_NUMBER\*\* ($tag/" data/testResults/README.md - done - - - name: Update local branch before committing changes + - name: Update local branch before committing (final) env: BRANCH_NAME: ${{ github.head_ref || github.ref_name }} run: | @@ -195,6 +246,17 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Now that the results (including model_qc_summary.md) are committed, post the + # comment from the committed summary. + - name: Post final comment + if: always() + uses: ./.github/actions/post-qc-comment + with: + mode: post + run-url: ${{ env.RUN_URL }} + base-ref: ${{ env.BASE_REF }} + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Upload full MEMOTE result uses: actions/upload-artifact@v7 with: @@ -209,12 +271,12 @@ jobs: run: | fail=0 [ "${{ steps.qc.outcome }}" = "failure" ] && fail=1 - for f in qc_roundtrip_cobra qc_roundtrip_raven qc_yamllint; do - [ "$(cat data/testResults/$f.txt 2>/dev/null)" = "fail" ] && { echo "::error::$f failed"; fail=1; } + for k in roundtrip_cobra roundtrip_raven yamllint; do + [ "$(python code/test/qcStatus.py --get $k)" = "fail" ] && { echo "::error::$k failed"; fail=1; } done - for f in qc_tasks_essential qc_tasks_verification; do - v=$(cat "data/testResults/$f.txt" 2>/dev/null) - [ -n "$v" ] && [ "${v%%/*}" != "0" ] && { echo "::error::$f: ${v%%/*} task(s) failed"; fail=1; } + for k in tasks_essential tasks_verification; do + v=$(python code/test/qcStatus.py --get $k) + [ -n "$v" ] && [ "${v%%/*}" != "0" ] && { echo "::error::$k: ${v%%/*} task(s) failed"; fail=1; } done if [ "$fail" = 1 ]; then echo "::error::A build gate failed; see the PR comment and the linked results." diff --git a/code/test/buildReport.py b/code/test/buildReport.py index 6c4bdb76..7c007991 100644 --- a/code/test/buildReport.py +++ b/code/test/buildReport.py @@ -1,9 +1,11 @@ """Build one model-quality report for the pull-request comment. Turns the result files under data/testResults/ into a single comment that leads -with a one-line verdict and then three status tables (structural checks, model QC -reports, MACAW/balance). Groups still being computed on this run are passed in the -RUNNING_GROUPS environment variable and their rows show as *running* (hourglass); +with a one-line verdict and then the status tables (model checks, MACAW/balance, +model-file/metabolic tasks, MEMOTE, gene essentiality). Each check name links to +its explanation in this folder's README. Groups still being computed on this run +are passed in the RUNNING_GROUPS environment variable and their rows show as +*running* (hourglass); the workflow calls this once with "all" before anything has run, once with "memote" while the slow MEMOTE snapshot is still going, and once with nothing when everything is in. No stamp files are involved. @@ -49,15 +51,34 @@ _running = os.environ.get("RUNNING_GROUPS", "") RUNNING = set(ALL_GROUPS) if _running.strip() == "all" else {g.strip() for g in _running.split(",") if g.strip()} + +def _slug(label: str) -> str: + """GitHub heading-anchor slug for a table label. Mirrors GitHub's algorithm + (lowercase, drop punctuation, spaces to hyphens) so a label links to the + same-named section in this folder's README.""" + s = label.lower().replace("`", "") + s = re.sub(r"[^\w\s-]", "", s) + return s.strip().replace(" ", "-") + + +def _labelled(label: str) -> str: + """The test name, linked to its explanation in the testResults README when the + repo URL is known (in CI); plain text when run locally.""" + return f"[{label}]({URL_BASE}/README.md#{_slug(label)})" if URL_BASE else label + # (label, key, kind, group, detail_file) -STRUCTURAL_ROWS = [ +# Structural gates and the model-QC reports share one table: the split between them +# was arbitrary (growth next to unused genes). The two gates (duplicate keys, growth) +# lead the table; every other row is a non-blocking report. Each label links to the +# matching section in this folder's README (see _labelled). +MODEL_ROWS = [ ("Duplicate `!!omap` keys", "dup_keys", "count", "checks", "qc_duplicate_keys.csv"), + ("Growth (biomass producible)", "growth", "growth", "checks", "qc_growth_blockers.csv"), ("Reactions with no metabolites", "empty_rxn", "count", "checks", "qc_empty_reactions.csv"), ("Model / annotation-table inconsistencies", "annot_consistency", "count", "checks", "qc_annotation_consistency.csv"), - ("Growth (biomass producible)", "growth", "growth", "checks", "qc_growth_blockers.csv"), -] -REPORT_ROWS = [ + ("Removed reactions or metabolites not deprecated", "removed_not_deprecated", "count", "checks", + "qc_deprecation_completeness.csv"), ("Metabolites missing formula", "missing_formula", "count", "checks", "qc_metabolite_completeness.csv"), ("Metabolites missing charge", "missing_charge", "count", "checks", "qc_metabolite_completeness.csv"), ("Reaction bound / GPR issues", "reaction_issues", "count", "checks", "qc_reaction_sanity.csv"), @@ -92,26 +113,57 @@ def _distinct_csv(path: Path, column: str) -> int | None: return len({row[column] for row in csv.DictReader(fh) if row.get(column)}) +def _status_map(directory: Path) -> dict: + """The combined qc_status.tsv as {check: result} ({} if absent). Holds the + one-line checks (round-trip, yamllint, metabolic tasks) and the growth value.""" + path = directory / "qc_status.tsv" + if not path.exists(): + return {} + out: dict[str, str] = {} + for line in path.read_text(encoding="utf-8").splitlines(): + parts = line.split("\t") + if len(parts) >= 2 and parts[0] != "check": + out[parts[0]] = parts[1] + return out + + def _growth(directory: Path) -> float | None: try: - return float((directory / "qc_growth.txt").read_text(encoding="utf-8").strip()) - except (FileNotFoundError, ValueError): + return float(_status_map(directory)["growth"]) + except (KeyError, ValueError): return None -def _memote_meta(directory: Path): - """Parse memote_score.md -> (total, mode, {section: score}, [(section, test, score)]), - or None if it has not been produced yet.""" +# memote_score.md is split into these two sections (see memoteSnapshot.py); each is +# parsed and compared only against the same section on the base branch, so a subset +# score is never diffed against a full-suite score. +MEMOTE_CORE = "Core subset" +MEMOTE_FULL = "Full suite" + + +def _memote_meta(directory: Path, title: str): + """Parse one section of memote_score.md -> + (total, mode, {section: score}, [(section, test, score)]), or None if that section + is absent or not yet computed (a placeholder with no total).""" path = directory / "memote_score.md" if not path.exists(): return None - text = path.read_text(encoding="utf-8") + full = path.read_text(encoding="utf-8") + m = re.search(rf"^## {re.escape(title)}\s*$(.*?)(?=^## |\Z)", full, re.M | re.S) + if m: + text = m.group(1) + elif title == MEMOTE_CORE: + text = full # back-compat: an older single-section file is the core subset + else: + return None total = re.search(r"Total score:\s*([\d.]+)\s*%", text) + if not total: + return None mode = re.search(r"Mode:\s*(.+?)\.", text) sections = {m.group(1): float(m.group(2)) for m in re.finditer(r"^\| (\w+) \| ([\d.]+)% \|$", text, re.M)} detailed = [(s, t, sc) for s, t, sc in re.findall(r"^\| (.+?) \| (.+?) \| ([\d.]+)% \|$", text, re.M)] - return (float(total.group(1)) if total else None, mode.group(1) if mode else "", sections, detailed) + return (float(total.group(1)), mode.group(1) if mode else "", sections, detailed) def _score_delta(cur, base) -> str: @@ -126,11 +178,12 @@ def _score_delta(cur, base) -> str: def _memote_section(current: Path, base: Path | None) -> str: if "memote" in RUNNING: return "_running_ · :hourglass_flowing_sand:" - meta = _memote_meta(current) - if meta is None: + core = _memote_meta(current, MEMOTE_CORE) + if core is None: return "_running_ · :hourglass_flowing_sand:" - total, mode, sections, detailed = meta - b = _memote_meta(base) if base and base.exists() else None + total, mode, sections, detailed = core + base_ok = base and base.exists() + b = _memote_meta(base, MEMOTE_CORE) if base_ok else None b_total, b_sections = (b[0], b[2]) if b else (None, {}) lines = [f"**Total score: {total:.1f}%** ({mode})   {_score_delta(total, b_total)}".rstrip(), ""] if sections: @@ -142,6 +195,17 @@ def _memote_section(current: Path, base: Path | None) -> str: "| Section | Test | Score |", "| --- | --- | ---: |"] lines += [f"| {s} | {t} | {sc}% |" for s, t, sc in detailed] lines += ["", ""] + + # Full suite: shown only if a /run memote result is committed. Compared to the + # full-suite section on the base branch, never to the subset score above. + full = _memote_meta(current, MEMOTE_FULL) + if full is not None: + bf = _memote_meta(base, MEMOTE_FULL) if base_ok else None + bf_total = bf[0] if bf else None + lines += ["", f"**Full suite: {full[0]:.1f}%**   {_score_delta(full[0], bf_total)} " + "· _from the last_ `/run memote`.".rstrip()] + else: + lines += ["", "_Full suite not run for this commit; comment_ `/run memote` _to add it._"] return "\n".join(lines) @@ -155,6 +219,7 @@ def _metrics(directory: Path) -> dict: "dup_keys": _count_csv(directory / "qc_duplicate_keys.csv"), "empty_rxn": _count_csv(directory / "qc_empty_reactions.csv"), "annot_consistency": _count_csv(directory / "qc_annotation_consistency.csv"), + "removed_not_deprecated": _count_csv(directory / "qc_deprecation_completeness.csv"), "growth": _growth(directory), "missing_formula": _count_csv(completeness, lambda r: r.get("missing_formula") == "yes"), "missing_charge": _count_csv(completeness, lambda r: r.get("missing_charge") == "yes"), @@ -215,25 +280,24 @@ def _table(rows, current: dict, base: dict): value = current.get(key) is_pending = value is None or group in RUNNING if is_pending: - lines.append(f"| {label} | _running_ | | :hourglass_flowing_sand: |") + lines.append(f"| {_labelled(label)} | _running_ | | :hourglass_flowing_sand: |") pending += 1 continue delta, icon, regression, row_fatal = _icon(value, base.get(key), kind) fatal = fatal or row_fatal or (key == "dup_keys" and value > 0) regressions += regression warnings += icon == ":warning:" - lines.append(f"| {label} | {_cell(value, kind, detail)} | {delta} | {icon} |") + lines.append(f"| {_labelled(label)} | {_cell(value, kind, detail)} | {delta} | {icon} |") return lines, regressions, warnings, pending, fatal -def _status(name: str) -> str: - p = RESULTS / f"qc_{name}.txt" - return p.read_text(encoding="utf-8").strip() if p.exists() else "" - - def _model_integrity_section() -> str: - """Round-trip, YAML lint and metabolic-task pass/fail from the status files the - workflow writes. A missing file means the check has not finished yet.""" + """Round-trip, YAML lint and metabolic-task pass/fail from the shared qc_status.tsv + the workflow writes. That file is committed, so it is present at checkout with + stale values from a previous run; it is only refreshed once the checks in the + early "checks" phase have run. While that phase is still going ("checks" in + RUNNING) show every row as running rather than the stale committed value; a + missing key likewise means the check has not finished yet.""" checks = [ ("YAML round-trip (cobrapy)", "roundtrip_cobra"), ("YAML round-trip (RAVEN)", "roundtrip_raven"), @@ -242,18 +306,20 @@ def _model_integrity_section() -> str: ("Verification metabolic tasks", "tasks_verification"), ] out = ["| Check | Result | |", "| --- | ---: | :---: |"] + pending = "checks" in RUNNING + status = {} if pending else _status_map(RESULTS) for label, name in checks: - val = _status(name) + val = status.get(name, "") if not val: - out.append(f"| {label} | _running_ | :hourglass_flowing_sand: |") + out.append(f"| {_labelled(label)} | _running_ | :hourglass_flowing_sand: |") elif "/" in val: # tasks: "failed/total" failed, total = val.split("/")[:2] ok = int(failed) == 0 - out.append(f"| {label} | {total + ' passed' if ok else failed + ' failed'} | " + out.append(f"| {_labelled(label)} | {total + ' passed' if ok else failed + ' failed'} | " f"{':white_check_mark:' if ok else ':x:'} |") else: # round-trip / lint: pass|fail ok = val.lower() == "pass" - out.append(f"| {label} | {val} | {':white_check_mark:' if ok else ':x:'} |") + out.append(f"| {_labelled(label)} | {val} | {':white_check_mark:' if ok else ':x:'} |") return "\n".join(out) @@ -270,13 +336,12 @@ def main() -> int: current = _metrics(RESULTS) base = _metrics(Path(BASE_DIR)) if have_base else {} - st_tbl, st_reg, st_warn, st_pend, fatal = _table(STRUCTURAL_ROWS, current, base) - rp_tbl, rp_reg, rp_warn, rp_pend, _ = _table(REPORT_ROWS, current, base) + md_tbl, md_reg, md_warn, md_pend, fatal = _table(MODEL_ROWS, current, base) mb_tbl, mb_reg, mb_warn, mb_pend, _ = _table(MB_ROWS, current, base) - regressions = st_reg + rp_reg + mb_reg - warnings = st_warn + rp_warn + mb_warn - pending = st_pend + rp_pend + mb_pend + regressions = md_reg + mb_reg + warnings = md_warn + mb_warn + pending = md_pend + mb_pend if fatal: verdict = ":x: **Merge blocked: the model cannot be loaded or cannot grow.** See the Structural checks table." @@ -299,14 +364,14 @@ def main() -> int: "", verdict, "", - "### Structural checks", - "_Duplicate keys (model unloadable) and no growth block the merge; the other rows are non-blocking._", - "", - head, sep, *st_tbl, + "_Each check name links to its explanation in the " + f"[testResults README]({URL_BASE}/README.md)._" if URL_BASE else "", "", - "### Model QC reports", + "### Model checks", + "_Duplicate keys (model unloadable) and no growth block the merge; every other row " + "is a non-blocking report._", "", - head, sep, *rp_tbl, + head, sep, *md_tbl, "", "### MACAW and mass/charge balance", "", @@ -316,14 +381,14 @@ def main() -> int: "", _model_integrity_section(), "", - "### MEMOTE", + f"### {_labelled('MEMOTE')}", "", _memote_section(RESULTS, Path(BASE_DIR) if BASE_DIR else None), "", "_The score above is the fast core subset. Comment_ `/run memote` " "_to run the full suite on this pull request; the score updates here when it finishes._", "", - "### Gene essentiality (Hart 2015)", + f"### {_labelled('Gene essentiality (Hart 2015)')}", "", _gene_essentiality_section(), "", diff --git a/code/test/memoteSnapshot.py b/code/test/memoteSnapshot.py index e80c47b4..ff1820b8 100644 --- a/code/test/memoteSnapshot.py +++ b/code/test/memoteSnapshot.py @@ -17,6 +17,9 @@ Writes the total score to data/testResults/memote_score.md (diff-friendly) and the scored result JSON to memote_result.json in the repository root, which the workflow uploads as a build artifact (it is not committed, to avoid bloating the repository). +memote_score.md keeps a "Core subset" and a "Full suite" section; each run rewrites +only its own section, so a routine subset run never overwrites a committed full-suite +score (see _write_section). Set GRB_LICENSE_FILE (a full Gurobi licence) to run with Gurobi; the genome-scale MILPs are impractical with GLPK. Without it the script falls back to the default @@ -39,6 +42,15 @@ RESULT_JSON = "memote_result.json" # repo root -> uploaded as artifact, not committed SCORE_MD = "data/testResults/memote_score.md" +# memote_score.md holds two independent sections. The fast core subset runs on every +# pull request and the full suite runs on demand (/run memote); they share the file +# but must not overwrite each other, so each run rewrites only its own section and +# leaves the other intact. A routine subset run therefore never erases a previously +# committed full-suite score, and buildReport compares each section only against the +# same section on the base branch (never subset vs full). +CORE_TITLE = "Core subset" +FULL_TITLE = "Full suite" + # The tests that dominate MEMOTE runtime on a genome-scale model. Two groups: # * consistency: MILP / flux-variability / per-metabolite optimisation over the # whole model (stoichiometric consistency, energy cycles, blocked reactions, @@ -146,6 +158,41 @@ def _detailed_rows(scored: dict, config) -> list[tuple[str, str, float]]: return rows +def _load_sections(path: str) -> dict: + """Existing memote_score.md as {section_title: body_text}. Empty if absent.""" + sections: dict[str, str] = {} + if not os.path.exists(path): + return sections + current, buf = None, [] + for line in open(path, encoding="utf-8").read().splitlines(): + if line.startswith("## "): + if current is not None: + sections[current] = "\n".join(buf).strip("\n") + current, buf = line[3:].strip(), [] + elif current is not None: + buf.append(line) + if current is not None: + sections[current] = "\n".join(buf).strip("\n") + return sections + + +def _placeholder(title: str) -> str: + if title == FULL_TITLE: + return "_Not run for this commit. Comment_ `/run memote` _to populate this section._" + return "_Not yet computed for this commit._" + + +def _write_section(this_title: str, body: str) -> None: + """Rewrite only this run's section, preserving the other one (or a placeholder).""" + sections = _load_sections(SCORE_MD) + sections[this_title] = body + out = ["# MEMOTE snapshot", ""] + for title in (CORE_TITLE, FULL_TITLE): + out += [f"## {title}", "", sections.get(title) or _placeholder(title), ""] + with open(SCORE_MD, "w", encoding="utf-8") as fh: + fh.write("\n".join(out).rstrip() + "\n") + + def main() -> int: subset = bool(os.environ.get("MEMOTE_SUBSET")) skip = SLOW_TESTS if subset else None @@ -186,7 +233,7 @@ def main() -> int: print("Scored MEMOTE result top-level keys:", sorted(scored.keys()), flush=True) total = _total_score(scored) - lines = ["# MEMOTE snapshot", "", f"Mode: {kind}."] + lines = [f"Mode: {kind}."] if subset: lines.append(f"Skipped (slow) tests: {', '.join(SLOW_TESTS)}.") lines.append("") @@ -213,8 +260,8 @@ def main() -> int: lines += [f"| {section} | {test} | {metric * 100:.1f}% |" for section, test, metric in detailed] - with open(SCORE_MD, "w", encoding="utf-8") as fh: - fh.write("\n".join(lines) + "\n") + # Rewrite only this run's section (core subset or full suite), keeping the other. + _write_section(CORE_TITLE if subset else FULL_TITLE, "\n".join(lines)) return 0 diff --git a/code/test/qcModelChecks.py b/code/test/qcModelChecks.py index 86cab93e..db25e1d0 100644 --- a/code/test/qcModelChecks.py +++ b/code/test/qcModelChecks.py @@ -19,19 +19,25 @@ - metabolites missing a formula or a charge; - reaction bound / GPR sanity; - exact-duplicate reactions (same stoichiometry); - - metabolites and genes not used by any reaction. + - metabolites and genes not used by any reaction; + - identifiers removed since the base branch that were not moved to a + deprecated list (needs BASE_MODEL_DIR; skipped when unavailable). Usage: python code/test/qcModelChecks.py """ import csv +import os import sys from collections import Counter, defaultdict +from pathlib import Path import cobra import yaml +import qcStatus + MODEL_FILE = "model/Human-GEM.yml" GENES_TSV = "model/genes.tsv" REACTIONS_TSV = "model/reactions.tsv" @@ -47,9 +53,16 @@ UNUSED_CSV = f"{RESULTS}/qc_unused_entities.csv" COMPLETENESS_CSV = f"{RESULTS}/qc_metabolite_completeness.csv" REACTION_SANITY_CSV = f"{RESULTS}/qc_reaction_sanity.csv" -GROWTH_TXT = f"{RESULTS}/qc_growth.txt" +DEPRECATION_COMPLETENESS_CSV = f"{RESULTS}/qc_deprecation_completeness.csv" +# Growth value goes into the shared qc_status.tsv (via qcStatus); only the +# variable-length list of blocking precursors keeps its own CSV. GROWTH_BLOCKERS_CSV = f"{RESULTS}/qc_growth_blockers.csv" +# Base-branch copies of reactions.tsv / metabolites.tsv, used to spot identifiers +# this pull request removed from the model. The workflow fetches them from the +# target branch; empty (check skipped) when run locally or on the first comparison. +BASE_MODEL_DIR = os.environ.get("BASE_MODEL_DIR", "") + GROWTH_TOLERANCE = 1e-6 # Pseudo-metabolites (generic class sinks and biomass pools) intrinsically @@ -189,6 +202,47 @@ def _numeric(value: str) -> bool: return issues +# --------------------------------------------------------------------------- # +# Report: removed identifiers must be moved to the deprecated lists +# --------------------------------------------------------------------------- # +def check_deprecation_completeness(model: cobra.Model) -> list[tuple]: + """Reactions/metabolites present in the base branch but gone from this model + must appear in the matching deprecated identifier file. Returns [(kind, id, issue)]. + + Human-GEM's convention is to retire identifiers, never silently delete them, so a + removed identifier stays resolvable. Detection needs the base-branch model tables + (BASE_MODEL_DIR); the check is skipped (empty result) when they are not available, + e.g. locally or on the first comparison for a branch. + """ + rows: list[tuple] = [] + if not BASE_MODEL_DIR or not Path(BASE_MODEL_DIR).exists(): + _write_csv(DEPRECATION_COMPLETENESS_CSV, ["kind", "id", "issue"], rows) + return rows + + current = { + "reaction": {r.id for r in model.reactions}, + "metabolite": {m.id for m in model.metabolites}, + } + # (kind, base table, id column, deprecated list, deprecated id column) + specs = [ + ("reaction", "reactions.tsv", "rxns", DEPRECATED_RXN_TSV, "rxns"), + ("metabolite", "metabolites.tsv", "mets", DEPRECATED_MET_TSV, "mets"), + ] + for kind, base_name, base_col, dep_tsv, dep_col in specs: + base_path = Path(BASE_MODEL_DIR) / base_name + if not base_path.exists(): + continue + base_ids = set(_tsv_column(str(base_path), base_col)) + deprecated = set(_tsv_column(dep_tsv, dep_col)) + removed = base_ids - current[kind] + for missing in sorted(removed - deprecated): + issue = f"removed from the model but not listed in {Path(dep_tsv).name}" + rows.append((kind, missing, issue)) + + _write_csv(DEPRECATION_COMPLETENESS_CSV, ["kind", "id", "issue"], rows) + return rows + + # --------------------------------------------------------------------------- # # Report: exact-duplicate reactions (identical stoichiometry) # --------------------------------------------------------------------------- # @@ -341,10 +395,14 @@ def main() -> int: print(f"::warning::{len(annotation)} model/annotation-table inconsistency(ies); " f"see {ANNOTATION_CONSISTENCY_CSV}.") + undeprecated = check_deprecation_completeness(model) + if undeprecated: + print(f"::warning::{len(undeprecated)} identifier(s) removed from the model but not " + f"added to a deprecated list; see {DEPRECATION_COMPLETENESS_CSV}.") + growth = check_growth(model) grows = growth == growth and growth > GROWTH_TOLERANCE # not NaN and positive - with open(GROWTH_TXT, "w", encoding="utf-8") as fh: - fh.write(f"{growth:.6g}\n") + qcStatus.set_status("growth", f"{growth:.6g}") if not grows: blockers = write_growth_blockers(model) print(f"::error::Model cannot produce biomass under its default constraints " @@ -364,6 +422,7 @@ def main() -> int: print(f"Reactions with bound/GPR issues: {n_reaction_issues}") print(f"Exact-duplicate reaction groups: {n_dup_rxn}") print(f"Unused metabolites / genes: {n_unused_met} / {n_unused_gene}") + print(f"Removed identifiers not deprecated: {len(undeprecated)}") print(f"Growth (max biomass, default constraints): {growth:.4g} " f"({'ok' if grows else 'NO GROWTH'})") diff --git a/code/test/qcStatus.py b/code/test/qcStatus.py new file mode 100644 index 00000000..ee9cd93c --- /dev/null +++ b/code/test/qcStatus.py @@ -0,0 +1,72 @@ +"""Read and write the combined QC status file (data/testResults/qc_status.tsv). + +Several fast checks each produce a single scalar - a pass/fail, a failed/total +count, or the growth value. Rather than commit a separate one-line file per check, +they all upsert into one key/value TSV: + + check result + growth 123.4 + roundtrip_cobra pass + roundtrip_raven pass + tasks_essential 0/57 + tasks_verification 0/21 + yamllint pass + +Upsert (read, set the one key, rewrite) keeps it order-independent and rerun-safe, +and because the QC steps run sequentially in one job there is no contention. Keys +are a fixed set, so nothing stale accumulates. + +CLI (used by the workflow's shell steps): + python code/test/qcStatus.py # set one key + python code/test/qcStatus.py --get # print one value (empty if unset) +""" + +import sys +from pathlib import Path + +STATUS_FILE = Path(__file__).resolve().parents[2] / "data" / "testResults" / "qc_status.tsv" +_HEADER = ("check", "result") + + +def read_status(path: Path = STATUS_FILE) -> dict: + """Return the status file as a {check: result} dict ({} if it does not exist).""" + if not path.exists(): + return {} + out: dict[str, str] = {} + for line in path.read_text(encoding="utf-8").splitlines(): + if not line.strip(): + continue + parts = line.split("\t") + if parts[0] == _HEADER[0]: # header row + continue + if len(parts) >= 2: + out[parts[0]] = parts[1] + return out + + +def set_status(key: str, value: str) -> None: + """Upsert one key and rewrite the file (header + keys sorted for a stable diff).""" + data = read_status() + data[key] = str(value) + STATUS_FILE.parent.mkdir(parents=True, exist_ok=True) + lines = ["\t".join(_HEADER)] + [f"{k}\t{data[k]}" for k in sorted(data)] + STATUS_FILE.write_text("\n".join(lines) + "\n", encoding="utf-8") + + +def get_status(key: str) -> str: + return read_status().get(key, "") + + +def main(argv: list[str]) -> int: + if len(argv) == 2 and argv[0] == "--get": + print(get_status(argv[1])) + return 0 + if len(argv) == 2: + set_status(argv[0], argv[1]) + return 0 + print("usage: qcStatus.py | --get ", file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/code/test/testMetabolicTasks.py b/code/test/testMetabolicTasks.py index a97b9ff7..44402322 100644 --- a/code/test/testMetabolicTasks.py +++ b/code/test/testMetabolicTasks.py @@ -17,6 +17,8 @@ from raven_toolbox.io import read_yaml_model from raven_toolbox.tasks import check_tasks +import qcStatus + # Repository root: this file is code/test/testMetabolicTasks.py REPO_ROOT = Path(__file__).resolve().parents[2] @@ -24,7 +26,6 @@ "essential": REPO_ROOT / "data" / "metabolicTasks" / "metabolicTasks_Essential.txt", "verification": REPO_ROOT / "data" / "metabolicTasks" / "metabolicTasks_VerifyModel.txt", } -STATUS_DIR = REPO_ROOT / "data" / "testResults" def _check_one(model, task_type: str) -> int: @@ -37,10 +38,8 @@ def _check_one(model, task_type: str) -> int: print(f"::error::Failed in {task_type} tasks ({len(failed)}/{len(results)} failed).") else: print(f"Succeeded with {task_type} tasks ({len(results)} passed).") - # one-line status for the QC comment - STATUS_DIR.mkdir(parents=True, exist_ok=True) - (STATUS_DIR / f"qc_tasks_{task_type}.txt").write_text( - f"{len(failed)}/{len(results)}\n", encoding="utf-8") + # one-line status for the QC comment, in the shared qc_status.tsv + qcStatus.set_status(f"tasks_{task_type}", f"{len(failed)}/{len(results)}") return 1 if failed else 0 diff --git a/data/testResults/README.md b/data/testResults/README.md index def0df92..1d7ee51a 100644 --- a/data/testResults/README.md +++ b/data/testResults/README.md @@ -1,56 +1,203 @@ # Test results -The file here contains results from the [MACAW](https://github.com/Devlin-Moyer/macaw) `dead_end_test` and `duplicate_test` tests, from a mass and charge balance report, and from cell-line specific gene essentiality prediction based on the [Hart _et al._ (2015)](https://doi.org/10.1016/j.cell.2015.11.015) dataset. - -The test results shown here were obtained by the GitHub Actions run in: - -- **PR #1027** (model QC checks) -- **PR #1027** (MEMOTE) -- **PR #1027** (MACAW and mass/charge balance) -- **PR #1027** (gene essentiality) - -The results will be updated by any subsequent pull request. Summary results are shown as a comment in the corresponding pull request. - -### MACAW: `dead_end_test` -Looks for metabolites in Human-GEM that can only be produced by all reactions they participate in or only consumed, then identifies all reactions that are prevented from sustaining steady-state fluxes because of each of these dead-end metabolites. The simplest case of a dead-end metabolite is one that only participates in a single reaction. Also flags all reversible reactions that can only carry fluxes in a single direction because one of their metabolites can either only be consumed or only be produced by all other reactions it participates in. - -### MACAW: `duplicate_test` -Identifies sets of reactions that may be duplicates of each other because they: - -- Involve exactly the same metabolites with exactly the same stoichiometric coefficients (but potentially different associated genes). -- Involve exactly the same metabolites, but go in different directions and/or some are reversible and some are not. -- Involve exactly the same metabolites, but with different stoichiometric coefficients. -- Represent the oxidation and/or reduction of the same metabolite, but use different electron acceptors/donors from the given list of pairs of oxidized and reduced forms of various electron carriers (e.g. NAD(H), NADP(H), FAD(H2), ubiquinone/ubiquinol, cytochromes). - -It is possible for a single reaction to fit in multiple of the above categories. There are sometimes cases where sets of reactions that fall into one of the above categories are completely legitimate representations of real biochemistry (e.g. separate irreversible reactions for importing vs exporting the same metabolite because two different transporters encoded by different genes are each responsible for transporting that metabolite in only one direction, enzymes that can use NAD(H) or NADP(H) interchangeably to catalyze the same redox reaction), but reactions that meet these criteria are generally worth close examination to ensure that they should actually all exist as separate reactions. - -### Mass and charge balance -Reports the reactions whose elemental (mass) or charge sums do not balance, using cobrapy's `check_mass_balance()`. Boundary reactions (exchange/demand/sink) and the biomass reaction are excluded, as they are not expected to balance. The unbalanced reactions are written to `balance_results.csv`, so a pull request that introduces a new imbalance is visible in the committed diff. This is a report and does not fail the build. - -### Cell-line specific gene essentiality -Evaluate gene essentiality predictions in 5 cell-line specific GEMs with experimental fitness data gathered from the [Hart _et al._ (2015)](https://doi.org/10.1016/j.cell.2015.11.015). - -Cell-line specific GEMs are constructed with tINIT2 for DLD1, GBM, HCT116, HeLa and RPE1 cell lines. Then, the `metabolicTasks_Essential.txt` list of tasks is used to identify essential genes in each of these models. The predicted gene essentiality is compared to results from a high-throughput CRISPR-Cas9 screen for identifying genes that affect fitness. Only the summary statistics of this comparison are kept. - -### Model QC checks -`code/test/qcModelChecks.py` (the `Model QC checks` workflow) runs the structural checks in one place. Each check writes a detailed, diff-friendly CSV so whatever is wrong is spelled out in a committed file, not only in the workflow log. - -**Build gates** (a finding fails the build; the model is unusable): - -- `qc_duplicate_keys.csv`: duplicate keys inside a metabolite/reaction/gene `!!omap` entry (two `name` fields, the same metabolite twice in a stoichiometry). RAVEN tolerates these, but `cobra.io.load_yaml_model` then raises a bare `AssertionError` with no location; the CSV names the entry, key and line numbers. The model cannot be loaded, so this stops the run. -- `qc_growth_blockers.csv`: when the model cannot produce biomass under its default constraints, the biomass precursors that cannot be made (empty when growth is fine). - -**Reports** (written to CSV and tracked with a delta versus the target branch; they do not fail the build, but a rising count shows as a regression in the comment): - -- `qc_empty_reactions.csv`: reactions with no metabolites. -- `qc_annotation_consistency.csv`: the model and its annotation tables (`reactions.tsv` / `metabolites.tsv` / `genes.tsv`) disagree, a deprecated identifier is used, or the `spontaneous` column is not numeric. -- `qc_metabolite_completeness.csv`: metabolites without a chemical formula or without a charge. Such metabolites are silently skipped by the mass/charge balance test, so tracking them keeps that test meaningful. -- `qc_reaction_sanity.csv`: reactions with invalid flux bounds (`lb > ub` or outside +/-1000) or GPR issues (genes not annotated in `genes.tsv`, or a boundary reaction with a gene rule). -- `qc_duplicate_reactions.csv`: reactions with identical stoichiometry (the strict "truly identical" duplicate). Near-duplicates (reverse direction, different coefficients, different electron carriers) are the remit of the MACAW `duplicate_test` above. -- `qc_unused_entities.csv`: metabolites and genes not used by any reaction. -- `qc_annotation_issues.csv` (from `annotationTest.py`): cross-reference problems in the annotation tables. Identifiers whose format does not match their namespace (KEGG, ChEBI, HMDB, PubChem, MetaNetX, Rhea, LipidMaps, EHMN, HepatoNET1, Reactome, TCDB), and metabolites whose cross-references are inconsistent across compartments. -- `memote_score.md`: the total score plus per-section and per-test scores from the [MEMOTE](https://memote.readthedocs.io) suite. MEMOTE is split by cost: every pull request runs a fast core subset (skipping the flux-variability, stoichiometric-consistency-MILP and matrix-rank tests that dominate runtime), and pull requests to `main` run the complete suite. The scored MEMOTE result is uploaded as a build artifact. - -The fast checks and MEMOTE run as two separate jobs, so the quick checks report without waiting for the (much slower) MEMOTE snapshot. When a gate fails, the detail is still committed and the comment still posted before the build is failed, so the failure is visible in both. - -All of the results are combined into a single pull-request comment (`model_qc_summary.md`): a one-line verdict (merge blocked / regressions / still running / clean), then a **Structural checks** table, a **Model QC reports** table, a **MACAW and mass/charge balance** table (each row: current value linked to its CSV, the change versus the target branch, and an icon), and the gene-essentiality metrics. The icon on each row is a red cross when a count rose versus the target branch (a regression this pull request introduced), a warning sign when a count is non-zero but did not rise (a pre-existing, non-blocking finding), and a check mark when the count is zero; growth is a check mark or cross, and the MEMOTE score warns only when it drops. Each result set is stamped with the commit it was computed for (`qc_checks.sha`, `qc_memote.sha`, `qc_macaw.sha`); a set whose stamp does not match the pull request's head commit has not run for the current commit, so its rows show as *running* (hourglass) rather than showing a previous run's numbers as current. +This folder holds the committed quality-control (QC) results for Human-GEM. Every +pull request re-runs the checks, commits the updated files here, and posts a summary +as a single comment on the pull request (`model_qc_summary.md`). Each check name in +that comment links to the matching explanation in section 2 below. + +The page has three parts: + +1. [Where the current results come from](#1-where-the-current-results-come-from) - which pull request produced each file. +2. [What each check means](#2-what-each-check-means) - the tests shown in the pull-request comment. +3. [Files in this folder](#3-files-in-this-folder) - what every file here contains. + +## 1. Where the current results come from + +Result files are regenerated and committed by GitHub Actions. Most are produced +together by the **Model QC** workflow, which runs on every pull request. The full +MEMOTE suite and the gene-essentiality prediction take hours, so they run on demand +only (by commenting `/run memote` or `/run gene-essentiality`) and update just their +own files. The pull request in each row is the one whose run last wrote those files. + +| Result file(s) | Produced by | Last updated by | +| --- | --- | --- | +| `qc_duplicate_keys.csv`, `qc_empty_reactions.csv`, `qc_annotation_consistency.csv`, `qc_deprecation_completeness.csv`, `qc_metabolite_completeness.csv`, `qc_reaction_sanity.csv`, `qc_duplicate_reactions.csv`, `qc_unused_entities.csv`, `qc_growth_blockers.csv` | `qcModelChecks.py` | **PR #1027** (model QC checks) | +| `qc_annotation_issues.csv` | `annotationTest.py` | **PR #1027** (model QC checks) | +| `qc_status.tsv` (round-trip, YAML lint, metabolic tasks, growth) | `testYamlConversion.py`, `testMetabolicTasks.py`, `action-yamllint`, `qcModelChecks.py` (via `qcStatus.py`) | **PR #1027** (model QC checks) | +| `macaw_results.csv`, `balance_results.csv`, `qc_structure_consistency.csv` | `macawTests.py`, `balanceTest.py`, `structureConsistencyTest.py` | **PR #1027** (MACAW and balance) | +| `memote_score.md` | `memoteSnapshot.py` (fast subset every PR; full suite via `/run memote`) | **PR #1027** (MEMOTE) | +| `gene-essential.csv`, `gene-essential_summary.md` | `geneEssentiality.py` via `/run gene-essentiality` | **PR #1027** (gene essentiality) | + +## 2. What each check means + +The headings below match the check names in the pull-request comment exactly, so a +name in the comment links straight to its explanation. + +In the comment, a count links to the CSV that lists the exact entries, and the icon +reads: :white_check_mark: the count is zero (or the model grows / the score is +non-zero); :warning: a non-zero but pre-existing finding that this pull request did +not make worse (non-blocking); :x: a count that rose versus the target branch (a +regression this pull request introduced), or a failed gate. + +### Model checks + +Structural integrity and per-entity quality, all from `qcModelChecks.py` unless +noted. Two rows are build gates (a finding blocks the merge); the rest are reports. + +#### Duplicate `!!omap` keys +**Gate.** Duplicate keys inside one metabolite/reaction/gene `!!omap` entry (two +`name` fields, or the same metabolite listed twice in a stoichiometry). RAVEN reads +and rewrites these, but `cobra.io.load_yaml_model` then raises a bare +`AssertionError` with no location, so the model cannot be loaded. The CSV names the +entry, key and line numbers. + +#### Growth (biomass producible) +**Gate.** Whether the model can produce biomass under its default constraints +(`slim_optimize`). When it cannot, `qc_growth_blockers.csv` lists the biomass +precursors that cannot be made, which are what to fix. + +#### Reactions with no metabolites +Reactions whose stoichiometry is empty. Such a reaction does nothing and usually +signals a broken edit. + +#### Model / annotation-table inconsistencies +The model and its annotation tables (`reactions.tsv` / `metabolites.tsv` / +`genes.tsv`) must list the same identifiers. Flags identifiers in the model but not +the table (or the reverse), any deprecated identifier still used in the model, and a +non-numeric value in the `spontaneous` column of `reactions.tsv`. + +#### Removed reactions or metabolites not deprecated +Human-GEM retires identifiers rather than deleting them, so a removed identifier +stays resolvable. This flags reactions or metabolites that are present on the target +branch but gone from this pull request's model and were **not** added to +`deprecatedReactions.tsv` / `deprecatedMetabolites.tsv`. A non-zero count means an +identifier was dropped without being moved to a deprecated list. (Comparison needs +the target-branch model tables, so it is reported only in CI.) + +#### Metabolites missing formula +Metabolites with no chemical formula. They are silently skipped by the mass-balance +test, so tracking them keeps that test meaningful. Generic pool/class +pseudo-metabolites, which have no formula by design, are excluded. + +#### Metabolites missing charge +Metabolites with no charge, for the same reason as the formula check. + +#### Reaction bound / GPR issues +Reactions with invalid flux bounds (`lb > ub`, or a bound outside +/-1000) or +gene-rule problems (a gene not annotated in `genes.tsv`, or a boundary reaction that +carries a gene rule). + +#### Exact-duplicate reaction groups +Groups of two or more reactions with **identical** stoichiometry (same metabolites +and same coefficients). This is the strict "truly identical" case; near-duplicates +(reverse direction, different coefficients, different electron carriers) are the +remit of the MACAW duplicate test below. + +#### Unused metabolites +Metabolites not used by any reaction in the model. + +#### Unused genes +Genes not referenced by any reaction's gene rule. + +#### Malformed cross-references +From `annotationTest.py`. Cross-references in the annotation tables whose format does +not match their namespace (KEGG, ChEBI, HMDB, PubChem, MetaNetX, Rhea, LipidMaps, +EHMN, HepatoNET1, Reactome, TCDB). + +#### Cross-refs inconsistent across compartments +From `annotationTest.py`. The same metabolite in different compartments carries +different cross-references, which should agree. + +### MACAW and mass/charge balance + +Network-level checks from [MACAW](https://github.com/Devlin-Moyer/macaw), the mass +and charge balance report, and the structure-vs-formula check. + +#### Reactions flagged by MACAW dead-end test +Reactions prevented from carrying steady-state flux because one of their metabolites +can only ever be produced, or only consumed, by every reaction it takes part in (the +simplest case being a metabolite in a single reaction). Also flags reversible +reactions that can therefore run in only one direction. + +#### Reactions flagged as MACAW duplicates +Sets of reactions that may be duplicates because they involve the same metabolites +(with the same or different coefficients or directions), or represent the same +oxidation/reduction using different electron carriers. Some are legitimate; the flag +means "worth checking", not "certainly wrong". + +#### Mass-imbalanced reactions +Reactions whose elemental sums do not balance, from cobrapy's `check_mass_balance()`. +Boundary reactions (exchange/demand/sink) and biomass are excluded, since they are +not expected to balance. + +#### Charge-imbalanced reactions +Reactions whose charge sums do not balance, with the same exclusions as above. + +#### Structure vs formula/charge inconsistencies +From `structureConsistencyTest.py`. Metabolites whose structure (SMILES/InChI in +`metabolites.tsv`) implies a formula or charge that disagrees with the formula/charge +carried in the model YAML. + +### Model file and metabolic tasks + +Whether the model file survives conversion and satisfies the curated task lists. + +#### YAML round-trip (cobrapy) +The model is loaded and re-written with cobrapy and must come back unchanged; a +failure means the YAML does not survive a cobrapy round-trip. **Gate.** + +#### YAML round-trip (RAVEN) +The same round-trip through the RAVEN toolbox. **Gate.** + +#### YAML lint +`yamllint` over `model/` (line-length rule disabled). **Gate.** + +#### Essential metabolic tasks +The number of `metabolicTasks_Essential.txt` tasks the model passes, from +`testMetabolicTasks.py`. Any failure blocks the merge. **Gate.** + +#### Verification metabolic tasks +The number of verification tasks the model passes, from `testMetabolicTasks.py`. Any +failure blocks the merge. **Gate.** + +### MEMOTE +The total score, plus per-section and per-test scores, from the +[MEMOTE](https://memote.readthedocs.io) suite (`memoteSnapshot.py`). Every pull +request runs a fast core subset (skipping the flux-variability, +stoichiometric-consistency MILP and matrix-rank tests that dominate runtime). +Comment `/run memote` to run the full suite; the score then updates in place. Higher +is better, so the comment warns only when the score drops versus the target branch. + +### Gene essentiality (Hart 2015) +Gene-essentiality predictions in five cell-line-specific GEMs (DLD1, GBM, HCT116, +HeLa, RPE1), built with tINIT2 and evaluated against the CRISPR-Cas9 fitness screen +of [Hart _et al._ (2015)](https://doi.org/10.1016/j.cell.2015.11.015). This takes +hours and is not run on every pull request; comment `/run gene-essentiality` to run +it, and the result posts as its own comment. Only the summary statistics of the +comparison are kept here. + +## 3. Files in this folder + +| File | Contents | +| --- | --- | +| `model_qc_summary.md` | The rendered pull-request comment (built by `buildReport.py` from the files below). Not a test itself. | +| `qc_status.tsv` | One key/value line each for the round-trip, YAML-lint and metabolic-task results and the growth value (see `qcStatus.py`). | +| `qc_duplicate_keys.csv` | Duplicate `!!omap` keys: entry, scope, key, first and duplicate line numbers. | +| `qc_growth_blockers.csv` | Biomass precursors that cannot be produced; empty when the model grows. | +| `qc_empty_reactions.csv` | Reactions with no metabolites. | +| `qc_annotation_consistency.csv` | Model-vs-annotation-table mismatches, deprecated-identifier use, and `spontaneous`-column problems: `kind, id, issue`. | +| `qc_deprecation_completeness.csv` | Reactions/metabolites removed since the target branch but not added to a deprecated list: `kind, id, issue`. | +| `qc_metabolite_completeness.csv` | Metabolites missing a formula and/or a charge: `metabolite, name, missing_formula, missing_charge`. | +| `qc_reaction_sanity.csv` | Reactions with bound or GPR issues: `reaction, name, issues`. | +| `qc_duplicate_reactions.csv` | Exact-duplicate reaction groups: `group, reaction, equation`. | +| `qc_unused_entities.csv` | Metabolites and genes used by no reaction: `kind, id`. | +| `qc_annotation_issues.csv` | Malformed and cross-compartment-inconsistent cross-references. | +| `qc_structure_consistency.csv` | Metabolites whose structure disagrees with the model formula/charge. | +| `macaw_results.csv` | Full MACAW output (dead-end and duplicate tests) per reaction. | +| `balance_results.csv` | Mass- and charge-imbalanced reactions. | +| `memote_score.md` | MEMOTE scores in two sections, core subset and full suite (see the MEMOTE explanation above). | +| `gene-essential.csv` | Per-gene essentiality matrix across the five cell-line models. | +| `gene-essential_summary.md` | Summary statistics of the gene-essentiality comparison against Hart 2015. | +| `README.md` | This file. | + + diff --git a/data/testResults/memote_score.md b/data/testResults/memote_score.md index 325a0524..a75708af 100644 --- a/data/testResults/memote_score.md +++ b/data/testResults/memote_score.md @@ -1,5 +1,7 @@ # MEMOTE snapshot +## Core subset + Mode: core subset. Skipped (slow) tests: test_stoichiometric_consistency, test_unconserved_metabolites, test_inconsistent_min_stoichiometry, test_detect_energy_generating_cycles, test_find_stoichiometrically_balanced_cycles, test_blocked_reactions, test_find_reactions_unbounded_flux_default_condition, test_find_metabolites_not_produced_with_open_bounds, test_find_metabolites_not_consumed_with_open_bounds, test_number_independent_conservation_relations, test_matrix_rank, test_degrees_of_freedom. @@ -46,3 +48,7 @@ Skipped (slow) tests: test_stoichiometric_consistency, test_unconserved_metaboli | Annotation - SBO Terms | Gene General SBO Presence | 100.0% | | Annotation - SBO Terms | Gene SBO:0000243 Presence | 100.0% | | Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 100.0% | + +## Full suite + +_Not run for this commit. Comment_ `/run memote` _to populate this section._ diff --git a/data/testResults/qc_deprecation_completeness.csv b/data/testResults/qc_deprecation_completeness.csv new file mode 100644 index 00000000..3d8d903e --- /dev/null +++ b/data/testResults/qc_deprecation_completeness.csv @@ -0,0 +1 @@ +kind,id,issue diff --git a/data/testResults/qc_growth.txt b/data/testResults/qc_growth.txt deleted file mode 100644 index 06d3fac1..00000000 --- a/data/testResults/qc_growth.txt +++ /dev/null @@ -1 +0,0 @@ -124.868 diff --git a/data/testResults/qc_roundtrip_cobra.txt b/data/testResults/qc_roundtrip_cobra.txt deleted file mode 100644 index 2ae28399..00000000 --- a/data/testResults/qc_roundtrip_cobra.txt +++ /dev/null @@ -1 +0,0 @@ -pass diff --git a/data/testResults/qc_roundtrip_raven.txt b/data/testResults/qc_roundtrip_raven.txt deleted file mode 100644 index 2ae28399..00000000 --- a/data/testResults/qc_roundtrip_raven.txt +++ /dev/null @@ -1 +0,0 @@ -pass diff --git a/data/testResults/qc_status.tsv b/data/testResults/qc_status.tsv new file mode 100644 index 00000000..dbef2fcc --- /dev/null +++ b/data/testResults/qc_status.tsv @@ -0,0 +1,7 @@ +check result +growth 124.868 +roundtrip_cobra pass +roundtrip_raven pass +tasks_essential 0/57 +tasks_verification 0/21 +yamllint pass diff --git a/data/testResults/qc_tasks_essential.txt b/data/testResults/qc_tasks_essential.txt deleted file mode 100644 index 9cfaf347..00000000 --- a/data/testResults/qc_tasks_essential.txt +++ /dev/null @@ -1 +0,0 @@ -0/57 diff --git a/data/testResults/qc_tasks_verification.txt b/data/testResults/qc_tasks_verification.txt deleted file mode 100644 index 4f19ed43..00000000 --- a/data/testResults/qc_tasks_verification.txt +++ /dev/null @@ -1 +0,0 @@ -0/21 diff --git a/data/testResults/qc_yamllint.txt b/data/testResults/qc_yamllint.txt deleted file mode 100644 index 2ae28399..00000000 --- a/data/testResults/qc_yamllint.txt +++ /dev/null @@ -1 +0,0 @@ -pass From df388e10d6b6f78ab96276f9f0d834ecd3f00590 Mon Sep 17 00:00:00 2001 From: edkerk <7326655+edkerk@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:52:21 +0000 Subject: [PATCH 02/11] chore: update model QC results (fast checks) [skip ci] --- data/testResults/README.md | 10 +-- data/testResults/model_qc_summary.md | 102 ++++++++------------------- 2 files changed, 34 insertions(+), 78 deletions(-) diff --git a/data/testResults/README.md b/data/testResults/README.md index 1d7ee51a..a2862854 100644 --- a/data/testResults/README.md +++ b/data/testResults/README.md @@ -21,11 +21,11 @@ own files. The pull request in each row is the one whose run last wrote those fi | Result file(s) | Produced by | Last updated by | | --- | --- | --- | -| `qc_duplicate_keys.csv`, `qc_empty_reactions.csv`, `qc_annotation_consistency.csv`, `qc_deprecation_completeness.csv`, `qc_metabolite_completeness.csv`, `qc_reaction_sanity.csv`, `qc_duplicate_reactions.csv`, `qc_unused_entities.csv`, `qc_growth_blockers.csv` | `qcModelChecks.py` | **PR #1027** (model QC checks) | -| `qc_annotation_issues.csv` | `annotationTest.py` | **PR #1027** (model QC checks) | -| `qc_status.tsv` (round-trip, YAML lint, metabolic tasks, growth) | `testYamlConversion.py`, `testMetabolicTasks.py`, `action-yamllint`, `qcModelChecks.py` (via `qcStatus.py`) | **PR #1027** (model QC checks) | -| `macaw_results.csv`, `balance_results.csv`, `qc_structure_consistency.csv` | `macawTests.py`, `balanceTest.py`, `structureConsistencyTest.py` | **PR #1027** (MACAW and balance) | -| `memote_score.md` | `memoteSnapshot.py` (fast subset every PR; full suite via `/run memote`) | **PR #1027** (MEMOTE) | +| `qc_duplicate_keys.csv`, `qc_empty_reactions.csv`, `qc_annotation_consistency.csv`, `qc_deprecation_completeness.csv`, `qc_metabolite_completeness.csv`, `qc_reaction_sanity.csv`, `qc_duplicate_reactions.csv`, `qc_unused_entities.csv`, `qc_growth_blockers.csv` | `qcModelChecks.py` | **PR #1062** (model QC checks) | +| `qc_annotation_issues.csv` | `annotationTest.py` | **PR #1062** (model QC checks) | +| `qc_status.tsv` (round-trip, YAML lint, metabolic tasks, growth) | `testYamlConversion.py`, `testMetabolicTasks.py`, `action-yamllint`, `qcModelChecks.py` (via `qcStatus.py`) | **PR #1062** (model QC checks) | +| `macaw_results.csv`, `balance_results.csv`, `qc_structure_consistency.csv` | `macawTests.py`, `balanceTest.py`, `structureConsistencyTest.py` | **PR #1062** (MACAW and balance) | +| `memote_score.md` | `memoteSnapshot.py` (fast subset every PR; full suite via `/run memote`) | **PR #1062** (MEMOTE) | | `gene-essential.csv`, `gene-essential_summary.md` | `geneEssentiality.py` via `/run gene-essentiality` | **PR #1027** (gene essentiality) | ## 2. What each check means diff --git a/data/testResults/model_qc_summary.md b/data/testResults/model_qc_summary.md index 281a485e..9f3c7613 100644 --- a/data/testResults/model_qc_summary.md +++ b/data/testResults/model_qc_summary.md @@ -2,98 +2,54 @@ :warning: **6 pre-existing finding(s), no regressions vs `develop`.** Non-blocking. -### Structural checks -_Duplicate keys (model unloadable) and no growth block the merge; the other rows are non-blocking._ +_Each check name links to its explanation in the [testResults README](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md)._ -| Check | Result | Δ vs `develop` | | -| --- | ---: | ---: | :---: | -| Duplicate `!!omap` keys | 0 | 0 | :white_check_mark: | -| Reactions with no metabolites | 0 | 0 | :white_check_mark: | -| Model / annotation-table inconsistencies | 0 | 0 | :white_check_mark: | -| Growth (biomass producible) | 125 | 0 | :white_check_mark: | - -### Model QC reports +### Model checks +_Duplicate keys (model unloadable) and no growth block the merge; every other row is a non-blocking report._ | Check | Result | Δ vs `develop` | | | --- | ---: | ---: | :---: | -| Metabolites missing formula | 0 | 0 | :white_check_mark: | -| Metabolites missing charge | 0 | 0 | :white_check_mark: | -| Reaction bound / GPR issues | 0 | 0 | :white_check_mark: | -| Exact-duplicate reaction groups | 0 | 0 | :white_check_mark: | -| Unused metabolites | 0 | 0 | :white_check_mark: | -| Unused genes | 0 | 0 | :white_check_mark: | -| Malformed cross-references | 0 | 0 | :white_check_mark: | -| Cross-refs inconsistent across compartments | [3](https://github.com/SysBioChalmers/Human-GEM/blob/worktree-matlab-to-python-workflows/data/testResults/qc_annotation_issues.csv) | 0 | :warning: | +| [Duplicate `!!omap` keys](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#duplicate-omap-keys) | 0 | 0 | :white_check_mark: | +| [Growth (biomass producible)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#growth-biomass-producible) | 125 | new | :white_check_mark: | +| [Reactions with no metabolites](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#reactions-with-no-metabolites) | 0 | 0 | :white_check_mark: | +| [Model / annotation-table inconsistencies](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#model--annotation-table-inconsistencies) | 0 | 0 | :white_check_mark: | +| [Removed reactions or metabolites not deprecated](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#removed-reactions-or-metabolites-not-deprecated) | 0 | new | :white_check_mark: | +| [Metabolites missing formula](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#metabolites-missing-formula) | 0 | 0 | :white_check_mark: | +| [Metabolites missing charge](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#metabolites-missing-charge) | 0 | 0 | :white_check_mark: | +| [Reaction bound / GPR issues](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#reaction-bound--gpr-issues) | 0 | 0 | :white_check_mark: | +| [Exact-duplicate reaction groups](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#exact-duplicate-reaction-groups) | 0 | 0 | :white_check_mark: | +| [Unused metabolites](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#unused-metabolites) | 0 | 0 | :white_check_mark: | +| [Unused genes](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#unused-genes) | 0 | 0 | :white_check_mark: | +| [Malformed cross-references](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#malformed-cross-references) | 0 | 0 | :white_check_mark: | +| [Cross-refs inconsistent across compartments](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#cross-refs-inconsistent-across-compartments) | [3](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/qc_annotation_issues.csv) | 0 | :warning: | ### MACAW and mass/charge balance | Check | Result | Δ vs `develop` | | | --- | ---: | ---: | :---: | -| Reactions flagged by MACAW dead-end test | [2510](https://github.com/SysBioChalmers/Human-GEM/blob/worktree-matlab-to-python-workflows/data/testResults/macaw_results.csv) | 0 | :warning: | -| Reactions flagged as MACAW duplicates | [377](https://github.com/SysBioChalmers/Human-GEM/blob/worktree-matlab-to-python-workflows/data/testResults/macaw_results.csv) | 0 | :warning: | -| Mass-imbalanced reactions | [87](https://github.com/SysBioChalmers/Human-GEM/blob/worktree-matlab-to-python-workflows/data/testResults/balance_results.csv) | 0 | :warning: | -| Charge-imbalanced reactions | [234](https://github.com/SysBioChalmers/Human-GEM/blob/worktree-matlab-to-python-workflows/data/testResults/balance_results.csv) | 0 | :warning: | -| Structure vs formula/charge inconsistencies | [397](https://github.com/SysBioChalmers/Human-GEM/blob/worktree-matlab-to-python-workflows/data/testResults/qc_structure_consistency.csv) | 0 | :warning: | +| [Reactions flagged by MACAW dead-end test](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#reactions-flagged-by-macaw-dead-end-test) | [2510](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/macaw_results.csv) | 0 | :warning: | +| [Reactions flagged as MACAW duplicates](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#reactions-flagged-as-macaw-duplicates) | [377](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/macaw_results.csv) | 0 | :warning: | +| [Mass-imbalanced reactions](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#mass-imbalanced-reactions) | [87](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/balance_results.csv) | 0 | :warning: | +| [Charge-imbalanced reactions](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#charge-imbalanced-reactions) | [234](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/balance_results.csv) | 0 | :warning: | +| [Structure vs formula/charge inconsistencies](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#structure-vs-formulacharge-inconsistencies) | [397](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/qc_structure_consistency.csv) | 0 | :warning: | ### Model file and metabolic tasks | Check | Result | | | --- | ---: | :---: | -| YAML round-trip (cobrapy) | pass | :white_check_mark: | -| YAML round-trip (RAVEN) | pass | :white_check_mark: | -| YAML lint | pass | :white_check_mark: | -| Essential metabolic tasks | 57 passed | :white_check_mark: | -| Verification metabolic tasks | 21 passed | :white_check_mark: | - -### MEMOTE - -**Total score: 20.2%** (core subset)   0 - -| Section | Score | Δ vs base | -| --- | ---: | ---: | -| consistency | 42.4% | 0 | -| annotation_met | 25.0% | 0 | -| annotation_rxn | 25.0% | 0 | -| annotation_gene | 0.0% | 0 | -| annotation_sbo | 0.0% | 0 | - -
Per-test scores +| [YAML round-trip (cobrapy)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#yaml-round-trip-cobrapy) | pass | :white_check_mark: | +| [YAML round-trip (RAVEN)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#yaml-round-trip-raven) | pass | :white_check_mark: | +| [YAML lint](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#yaml-lint) | pass | :white_check_mark: | +| [Essential metabolic tasks](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#essential-metabolic-tasks) | 57 passed | :white_check_mark: | +| [Verification metabolic tasks](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#verification-metabolic-tasks) | 21 passed | :white_check_mark: | -| Section | Test | Score | -| --- | --- | ---: | -| Consistency | Stoichiometric Consistency | 100.0% | -| Consistency | Mass Balance | 0.8% | -| Consistency | Charge Balance | 2.1% | -| Consistency | Metabolite Connectivity | 0.0% | -| Consistency | Unbounded Flux In Default Medium | 100.0% | -| Annotation - Metabolites | Presence of Metabolite Annotation | 100.0% | -| Annotation - Metabolites | Metabolite Annotations Per Database | 100.0% | -| Annotation - Metabolites | Metabolite Annotation Conformity Per Database | 100.0% | -| Annotation - Metabolites | Uniform Metabolite Identifier Namespace | 0.0% | -| Annotation - Reactions | Presence of Reaction Annotation | 100.0% | -| Annotation - Reactions | Reaction Annotations Per Database | 100.0% | -| Annotation - Reactions | Reaction Annotation Conformity Per Database | 100.0% | -| Annotation - Reactions | Uniform Reaction Identifier Namespace | 0.0% | -| Annotation - Genes | Presence of Gene Annotation | 100.0% | -| Annotation - Genes | Gene Annotations Per Database | 100.0% | -| Annotation - Genes | Gene Annotation Conformity Per Database | 100.0% | -| Annotation - SBO Terms | Metabolite General SBO Presence | 100.0% | -| Annotation - SBO Terms | Metabolite SBO:0000247 Presence | 100.0% | -| Annotation - SBO Terms | Reaction General SBO Presence | 100.0% | -| Annotation - SBO Terms | Metabolic Reaction SBO:0000176 Presence | 100.0% | -| Annotation - SBO Terms | Transport Reaction SBO:0000185 Presence | 100.0% | -| Annotation - SBO Terms | Exchange Reaction SBO:0000627 Presence | 100.0% | -| Annotation - SBO Terms | Demand Reaction SBO:0000628 Presence | 100.0% | -| Annotation - SBO Terms | Sink Reactions SBO:0000632 Presence | 100.0% | -| Annotation - SBO Terms | Gene General SBO Presence | 100.0% | -| Annotation - SBO Terms | Gene SBO:0000243 Presence | 100.0% | -| Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 100.0% | +### [MEMOTE](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#memote) -
+_running_ · :hourglass_flowing_sand: _The score above is the fast core subset. Comment_ `/run memote` _to run the full suite on this pull request; the score updates here when it finishes._ -### Gene essentiality (Hart 2015) +### [Gene essentiality (Hart 2015)](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#gene-essentiality-hart-2015) _Not run automatically (it takes hours). Comment_ `/run gene-essentiality` _to run it on this pull request; the result posts as its own comment._ From 3dfc3bda0a1a1f1faf1c3a85f1eb471660ec0a13 Mon Sep 17 00:00:00 2001 From: edkerk <7326655+edkerk@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:55:07 +0000 Subject: [PATCH 03/11] chore: update model QC results [skip ci] --- data/testResults/model_qc_summary.md | 46 +++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/data/testResults/model_qc_summary.md b/data/testResults/model_qc_summary.md index 9f3c7613..a18aa0b1 100644 --- a/data/testResults/model_qc_summary.md +++ b/data/testResults/model_qc_summary.md @@ -45,7 +45,51 @@ _Duplicate keys (model unloadable) and no growth block the merge; every other ro ### [MEMOTE](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#memote) -_running_ · :hourglass_flowing_sand: +**Total score: 20.2%** (core subset)   0 + +| Section | Score | Δ vs base | +| --- | ---: | ---: | +| consistency | 42.4% | 0 | +| annotation_met | 25.0% | 0 | +| annotation_rxn | 25.0% | 0 | +| annotation_gene | 0.0% | 0 | +| annotation_sbo | 0.0% | 0 | + +
Per-test scores + +| Section | Test | Score | +| --- | --- | ---: | +| Consistency | Stoichiometric Consistency | 100.0% | +| Consistency | Mass Balance | 0.8% | +| Consistency | Charge Balance | 2.1% | +| Consistency | Metabolite Connectivity | 0.0% | +| Consistency | Unbounded Flux In Default Medium | 100.0% | +| Annotation - Metabolites | Presence of Metabolite Annotation | 100.0% | +| Annotation - Metabolites | Metabolite Annotations Per Database | 100.0% | +| Annotation - Metabolites | Metabolite Annotation Conformity Per Database | 100.0% | +| Annotation - Metabolites | Uniform Metabolite Identifier Namespace | 0.0% | +| Annotation - Reactions | Presence of Reaction Annotation | 100.0% | +| Annotation - Reactions | Reaction Annotations Per Database | 100.0% | +| Annotation - Reactions | Reaction Annotation Conformity Per Database | 100.0% | +| Annotation - Reactions | Uniform Reaction Identifier Namespace | 0.0% | +| Annotation - Genes | Presence of Gene Annotation | 100.0% | +| Annotation - Genes | Gene Annotations Per Database | 100.0% | +| Annotation - Genes | Gene Annotation Conformity Per Database | 100.0% | +| Annotation - SBO Terms | Metabolite General SBO Presence | 100.0% | +| Annotation - SBO Terms | Metabolite SBO:0000247 Presence | 100.0% | +| Annotation - SBO Terms | Reaction General SBO Presence | 100.0% | +| Annotation - SBO Terms | Metabolic Reaction SBO:0000176 Presence | 100.0% | +| Annotation - SBO Terms | Transport Reaction SBO:0000185 Presence | 100.0% | +| Annotation - SBO Terms | Exchange Reaction SBO:0000627 Presence | 100.0% | +| Annotation - SBO Terms | Demand Reaction SBO:0000628 Presence | 100.0% | +| Annotation - SBO Terms | Sink Reactions SBO:0000632 Presence | 100.0% | +| Annotation - SBO Terms | Gene General SBO Presence | 100.0% | +| Annotation - SBO Terms | Gene SBO:0000243 Presence | 100.0% | +| Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 100.0% | + +
+ +_Full suite not run for this commit; comment_ `/run memote` _to add it._ _The score above is the fast core subset. Comment_ `/run memote` _to run the full suite on this pull request; the score updates here when it finishes._ From 84a2469585b7f344dd8bcc34ab18fc2a27e336c7 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Wed, 15 Jul 2026 23:16:05 +0200 Subject: [PATCH 04/11] ci(memote): enrich the model with table cross-references before MEMOTE The committed YAML model carries only ids and names, so MEMOTE scored every annotation section 0% even though the cross-references exist in the annotation tables. Add annotateModel.py, which attaches the database identifiers from metabolites.tsv / reactions.tsv / genes.tsv to an in-memory model, and call it in memoteSnapshot.py before writing the temporary SBML. - Maps only registry (identifiers.org) namespaces MEMOTE can validate; legacy-only columns (EHMN, HepatoNET1, Recon3D, HMR2, Ratcon) are skipped. - Normalises values to each namespace: Rhea loses its "RHEA:" prefix, KEGG metabolite ids split into compound/glycan/drug by prefix, genes get ensembl (from the id) plus uniprot and ncbigene. - The enriched model exists only in memory for the temporary SBML; nothing extra is committed. --- code/test/annotateModel.py | 127 ++++++++++++++++++++++++++++++++++++ code/test/memoteSnapshot.py | 16 +++++ data/testResults/README.md | 6 ++ 3 files changed, 149 insertions(+) create mode 100644 code/test/annotateModel.py diff --git a/code/test/annotateModel.py b/code/test/annotateModel.py new file mode 100644 index 00000000..f44b4954 --- /dev/null +++ b/code/test/annotateModel.py @@ -0,0 +1,127 @@ +"""Attach database cross-references from the annotation tables to a cobra model. + +The committed YAML model carries only ids and names; the cross-references live in +model/metabolites.tsv, model/reactions.tsv and model/genes.tsv. MEMOTE reads +annotations from the SBML export, so without them it scores every annotation test +0% even though Human-GEM has the cross-references. This enriches an in-memory model +- used only to build the temporary SBML that MEMOTE runs on, never committed - so +the MEMOTE annotation scores reflect the identifiers Human-GEM actually carries. + +Only registry (identifiers.org) namespaces MEMOTE can validate are attached, and +values are normalised to each namespace's expected form (e.g. Rhea drops its +"RHEA:" prefix; KEGG metabolite ids are split into compound/glycan/drug by prefix). +Legacy-only columns (EHMN, HepatoNET1, Recon3D, HMR2, Ratcon) are skipped. + +Usage: + import annotateModel + annotateModel.enrich(model) # mutates the model in place +""" + +import csv +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +METABOLITES_TSV = REPO_ROOT / "model" / "metabolites.tsv" +REACTIONS_TSV = REPO_ROOT / "model" / "reactions.tsv" +GENES_TSV = REPO_ROOT / "model" / "genes.tsv" + +# TSV column -> (identifiers.org namespace, prefix to strip so the value conforms). +MET_MAP = { + "metBiGGID": ("bigg.metabolite", None), + "metHMDBID": ("hmdb", None), + "metChEBIID": ("chebi", None), # values already "CHEBI:1234" + "metPubChemID": ("pubchem.compound", None), + "metLipidMapsID": ("lipidmaps", None), + "metMetaNetXID": ("metanetx.chemical", None), + "metSeedID": ("seed.compound", None), +} +RXN_MAP = { + "rxnKEGGID": ("kegg.reaction", None), + "rxnBiGGID": ("bigg.reaction", None), + "rxnMetaNetXID": ("metanetx.reaction", None), + "rxnRheaID": ("rhea", "RHEA:"), # identifiers.org rhea wants the bare number + "rxnREACTOMEID": ("reactome", None), + "rxnTCDBID": ("tcdb", None), +} +GENE_MAP = { + "geneUniProtID": ("uniprot", None), + "geneEntrezID": ("ncbigene", None), +} +# KEGG metabolite ids span three namespaces, told apart by their first letter. +KEGG_MET_NS = {"C": "kegg.compound", "G": "kegg.glycan", "D": "kegg.drug"} + + +def _read(path: Path) -> list[dict]: + with open(path, newline="", encoding="utf-8") as fh: + return list(csv.DictReader(fh, delimiter="\t")) + + +def _values(raw: str, strip_prefix) -> list[str]: + parts = [p.strip() for p in (raw or "").split(";") if p.strip()] + if strip_prefix: + parts = [p[len(strip_prefix):] if p.startswith(strip_prefix) else p for p in parts] + return parts + + +def _add(ann: dict, namespace: str, value: str) -> None: + """Add one identifier under a namespace, keeping a list when there are several.""" + cur = ann.get(namespace) + if cur is None: + ann[namespace] = value + elif isinstance(cur, list): + if value not in cur: + cur.append(value) + elif cur != value: + ann[namespace] = [cur, value] + + +def _apply(entity, row: dict, mapping: dict) -> bool: + ann = dict(entity.annotation or {}) + before = len(ann) + for column, (namespace, strip) in mapping.items(): + for value in _values(row.get(column, ""), strip): + _add(ann, namespace, value) + entity.annotation = ann + return len(ann) > before + + +def enrich(model) -> dict: + """Attach cross-references to every metabolite/reaction/gene that has a table + row. Returns {'metabolites': n, 'reactions': n, 'genes': n} annotated.""" + mets = {r["mets"]: r for r in _read(METABOLITES_TSV)} + rxns = {r["rxns"]: r for r in _read(REACTIONS_TSV)} + genes = {r["genes"]: r for r in _read(GENES_TSV)} + counts = {"metabolites": 0, "reactions": 0, "genes": 0} + + for met in model.metabolites: + row = mets.get(met.id) + if not row: + continue + touched = _apply(met, row, MET_MAP) + ann = dict(met.annotation or {}) + for value in _values(row.get("metKEGGID", ""), None): + namespace = KEGG_MET_NS.get(value[:1]) + if namespace: + _add(ann, namespace, value) + touched = True + met.annotation = ann + counts["metabolites"] += touched + + for rxn in model.reactions: + row = rxns.get(rxn.id) + if row and _apply(rxn, row, RXN_MAP): + counts["reactions"] += 1 + + for gene in model.genes: + ann = dict(gene.annotation or {}) + # The gene id is itself the Ensembl gene identifier. + if gene.id.startswith("ENSG"): + _add(ann, "ensembl", gene.id) + gene.annotation = ann + touched = bool(ann) + row = genes.get(gene.id) + if row: + touched = _apply(gene, row, GENE_MAP) or touched + counts["genes"] += touched + + return counts diff --git a/code/test/memoteSnapshot.py b/code/test/memoteSnapshot.py index ff1820b8..e24d1e40 100644 --- a/code/test/memoteSnapshot.py +++ b/code/test/memoteSnapshot.py @@ -21,6 +21,11 @@ only its own section, so a routine subset run never overwrites a committed full-suite score (see _write_section). +Before exporting the SBML, the model is enriched with the database cross-references +from the annotation tables (see annotateModel.py) so MEMOTE's annotation tests score +against the identifiers Human-GEM actually carries. The enriched model exists only in +memory for the temporary SBML; it is never committed. + Set GRB_LICENSE_FILE (a full Gurobi licence) to run with Gurobi; the genome-scale MILPs are impractical with GLPK. Without it the script falls back to the default solver. @@ -38,6 +43,8 @@ import memote.suite.api as api from memote.suite.reporting import ReportConfiguration, SnapshotReport +import annotateModel + MODEL_FILE = "model/Human-GEM.yml" RESULT_JSON = "memote_result.json" # repo root -> uploaded as artifact, not committed SCORE_MD = "data/testResults/memote_score.md" @@ -206,6 +213,15 @@ def main() -> int: # memote reads an SBML model, so convert the canonical YAML model to a # temporary SBML file first (memote fails on a .yml directly). model = cobra.io.load_yaml_model(MODEL_FILE) + + # The YAML model has only ids and names; attach the database cross-references + # from the annotation tables so MEMOTE's annotation tests see them. This mutates + # the in-memory model only - the SBML written below is temporary and the enriched + # model is never committed. + counts = annotateModel.enrich(model) + print(f"Annotated for MEMOTE (temporary): {counts['metabolites']} metabolites, " + f"{counts['reactions']} reactions, {counts['genes']} genes.", flush=True) + sbml_path = os.path.join(tempfile.gettempdir(), "human-gem.xml") cobra.io.write_sbml_model(model, sbml_path) diff --git a/data/testResults/README.md b/data/testResults/README.md index a2862854..a571e8d0 100644 --- a/data/testResults/README.md +++ b/data/testResults/README.md @@ -168,6 +168,12 @@ stoichiometric-consistency MILP and matrix-rank tests that dominate runtime). Comment `/run memote` to run the full suite; the score then updates in place. Higher is better, so the comment warns only when the score drops versus the target branch. +Before running, the model is enriched with the database cross-references from the +annotation tables (`annotateModel.py`), so the annotation tests score against the +identifiers Human-GEM actually carries rather than the bare ids in the YAML. The +enriched model is used only to build the temporary SBML MEMOTE reads; it is not +committed. The score is stored in two sections, `Core subset` and `Full suite`. + ### Gene essentiality (Hart 2015) Gene-essentiality predictions in five cell-line-specific GEMs (DLD1, GBM, HCT116, HeLa, RPE1), built with tINIT2 and evaluated against the CRISPR-Cas9 fitness screen From a39784752433c097dd56c7992df678385df130bc Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Wed, 15 Jul 2026 23:31:17 +0200 Subject: [PATCH 05/11] ci(memote): use canonical annotateGEM (with extended SBO) to enrich for MEMOTE Bring the canonical annotation helper code/annotateGEM.py (a port of annotateGEM.m) and its release caller code/io/increaseHumanGEMVersion.py into the repo, and use annotate_gem from memoteSnapshot.py instead of an ad-hoc helper. This attaches the TSV cross-references and SBO terms to the in-memory model before the temporary SBML that MEMOTE reads (nothing extra is committed). Extend annotateGEM's SBO assignment to everything MEMOTE checks: - metabolites get SBO:0000247 (simple chemical) and genes SBO:0000243 (gene); - boundary reactions split into exchange / demand / sink (SBO:0000627 / 0000628 / 0000632) via cobra's own classification, which MEMOTE also uses, so each reaction carries the term its check expects. Falls back to exchange-for-all if cobra cannot classify. Remove the interim code/test/annotateModel.py in favour of annotateGEM. --- code/annotateGEM.py | 236 +++++++++++++++++++++++++++++ code/io/increaseHumanGEMVersion.py | 211 ++++++++++++++++++++++++++ code/test/annotateModel.py | 127 ---------------- code/test/memoteSnapshot.py | 31 ++-- data/testResults/README.md | 11 +- 5 files changed, 472 insertions(+), 144 deletions(-) create mode 100644 code/annotateGEM.py create mode 100644 code/io/increaseHumanGEMVersion.py delete mode 100644 code/test/annotateModel.py diff --git a/code/annotateGEM.py b/code/annotateGEM.py new file mode 100644 index 00000000..38c1154d --- /dev/null +++ b/code/annotateGEM.py @@ -0,0 +1,236 @@ +"""Attach cross-reference (MIRIAM) and SBO annotation to a Human-GEM model. + +Python/raven-toolbox port of ``code/annotateGEM.m``. + +The YAML model stores only the inline fields (``eccodes``, ``metFrom``, +``smiles``); the full set of external identifiers lives in the annotation +tables ``model/reactions.tsv``, ``model/metabolites.tsv`` and +``model/genes.tsv``. This module reads those tables and writes the identifiers +onto each cobra entity's ``annotation`` dict (namespace -> list of ids), and +assigns an SBO term to every reaction (classified into +biochemical/transport/exchange/demand/sink/biomass), metabolite (simple chemical) +and gene. The exported SBML / Excel / txt then carry the annotation, while the YAML +and ``.mat`` files stay annotation-light (their cross-references remain the TSV +tables), exactly as the MATLAB release flow produces them. + +Used by ``code/io/increaseHumanGEMVersion.py``; can also be run standalone to +inspect the merge: + + python code/annotateGEM.py # counts only, model not modified on disk +""" +from __future__ import annotations + +from pathlib import Path + +import cobra +import pandas as pd + +# Map TSV column names to identifiers.org namespaces (from annotateGEM.m id2miriam). +_RXN_ID2MIRIAM = { + "rxnKEGGID": "kegg.reaction", + "rxnBiGGID": "bigg.reaction", + "rxnREACTOMEID": "reactome", + "rxnRecon3DID": "vmhreaction", + "rxnMetaNetXID": "metanetx.reaction", + "rxnTCDBID": "tcdb", + "rxnRheaID": "rhea", + "rxnRheaMasterID": "rhea", +} +_MET_ID2MIRIAM = { + "metBiGGID": "bigg.metabolite", + "metKEGGID": "kegg.compound", + "metHMDBID": "hmdb", + "metChEBIID": "chebi", + "metPubChemID": "pubchem.compound", + "metLipidMapsID": "lipidmaps", + "metRecon3DID": "vmhmetabolite", + "metMetaNetXID": "metanetx.chemical", + "metSeedID": "seed.compound", +} +_GENE_ID2MIRIAM = { + "genes": "ensembl", + "geneENSTID": "ensembl", + "geneENSPID": "ensembl", + "geneUniProtID": "uniprot", + "geneSymbols": "hgnc.symbol", + "geneEntrezID": "ncbigene", +} + +# Reaction SBO terms (annotateGEM.m). Precedence low -> high: default, biomass, +# transport, then boundary (a later match overrides an earlier one). Boundary +# reactions are split into exchange/demand/sink, matching how cobra (and hence +# MEMOTE) classifies them, so each gets the SBO term its check expects. +_SBO_DEFAULT = "SBO:0000176" # biochemical reaction +_SBO_BIOMASS = "SBO:0000629" # biomass production +_SBO_TRANSPORT = "SBO:0000185" # translocation reaction +_SBO_EXCHANGE = "SBO:0000627" # exchange reaction +_SBO_DEMAND = "SBO:0000628" # demand reaction +_SBO_SINK = "SBO:0000632" # sink reaction +# Metabolite and gene SBO terms (one each; no classification needed). +_SBO_METABOLITE = "SBO:0000247" # simple chemical +_SBO_GENE = "SBO:0000243" # gene + + +def _read_tsv(path: Path) -> pd.DataFrame: + """Read a TSV annotation table as text (empty cells become ``""``).""" + return pd.read_csv(path, sep="\t", dtype=str, keep_default_na=False) + + +def _split_ids(cell: str) -> list[str]: + """Split a ``";"``-separated annotation cell into clean, non-empty ids.""" + return [part.strip() for part in str(cell).split(";") if part.strip()] + + +def _chebi(ids: list[str]) -> list[str]: + """Ensure every ChEBI id has the ``CHEBI:`` prefix (annotateGEM.m rule).""" + out = [] + for i in ids: + out.append(i if i.upper().startswith("CHEBI:") else f"CHEBI:{i}") + return out + + +def _rhea(ids: list[str]) -> list[str]: + """Strip the ``RHEA:`` prefix; it must not appear in the identifiers.org URL.""" + return [i[5:] if i.upper().startswith("RHEA:") else i for i in ids] + + +def _apply_row(annotation: dict, row: pd.Series, id2miriam: dict) -> None: + """Add the mapped id columns of ``row`` to ``annotation`` (namespace -> list).""" + for column, namespace in id2miriam.items(): + if column not in row: + continue + ids = _split_ids(row[column]) + if namespace == "chebi": + ids = _chebi(ids) + elif namespace == "rhea": + ids = _rhea(ids) + if not ids: + continue + merged = list(annotation.get(namespace, [])) + merged.extend(ids) + # Dedupe while preserving first-seen order (columns can share a namespace). + annotation[namespace] = list(dict.fromkeys(merged)) + + +def _is_transport(rxn: cobra.Reaction) -> bool: + """True if a metabolite name appears in more than one compartment (RAVEN + getTransportRxns): the reaction moves a species across compartments.""" + comps_by_name: dict[str, set[str]] = {} + for met in rxn.metabolites: + comps_by_name.setdefault(met.name, set()).add(met.compartment) + return any(len(comps) > 1 for comps in comps_by_name.values()) + + +def _boundary_sbo(model: cobra.Model) -> dict: + """Map each boundary reaction to its SBO term (exchange / demand / sink). + + Uses cobra's own ``exchanges`` / ``demands`` / ``sinks`` classification, which + MEMOTE also uses, so the assigned term matches the check MEMOTE will apply. If + cobra cannot classify (e.g. it fails to find an external compartment, or the + model type lacks these properties), fall back to treating every boundary + reaction as an exchange, as the original port did.""" + try: + exchanges, demands, sinks = model.exchanges, model.demands, model.sinks + except Exception: # noqa: BLE001 - any classification failure -> safe fallback + exchanges, demands, sinks = model.boundary, [], [] + sbo = {} + for rxn in exchanges: + sbo[rxn.id] = _SBO_EXCHANGE + for rxn in demands: + sbo[rxn.id] = _SBO_DEMAND + for rxn in sinks: + sbo[rxn.id] = _SBO_SINK + # Any boundary reaction cobra did not place lands as an exchange. + for rxn in model.boundary: + sbo.setdefault(rxn.id, _SBO_EXCHANGE) + return sbo + + +def _assign_sbo(model: cobra.Model) -> None: + """Set ``annotation['sbo']`` on every reaction (annotateGEM.m SBO logic).""" + boundary_sbo = _boundary_sbo(model) + for rxn in model.reactions: + sbo = _SBO_DEFAULT + is_biomass = "biomass" in rxn.id.lower() or any( + met.name.lower() == "biomass" and coeff > 0 + for met, coeff in rxn.metabolites.items() + ) + if is_biomass: + sbo = _SBO_BIOMASS + if _is_transport(rxn): + sbo = _SBO_TRANSPORT + if rxn.id in boundary_sbo: + sbo = boundary_sbo[rxn.id] + rxn.annotation["sbo"] = sbo + + +def annotate_gem( + model: cobra.Model, + model_dir: str | Path, + *, + types: tuple[str, ...] = ("rxn", "met", "gene"), +) -> cobra.Model: + """Merge the TSV cross-references and SBO terms into ``model`` in place. + + Parameters + ---------- + model + Model whose reactions/metabolites/genes carry Human-GEM ids. + model_dir + Directory holding ``reactions.tsv`` / ``metabolites.tsv`` / ``genes.tsv``. + types + Which annotation classes to add (``"rxn"``, ``"met"``, ``"gene"``). + + Returns + ------- + cobra.Model + The same ``model`` object, now annotated. Pass a copy if the caller + needs to keep an un-annotated version (the release keeps the YAML/.mat + exports annotation-light this way). + """ + model_dir = Path(model_dir) + + if "met" in types: + mets = _read_tsv(model_dir / "metabolites.tsv").set_index("mets") + for met in model.metabolites: + if met.id in mets.index: + _apply_row(met.annotation, mets.loc[met.id], _MET_ID2MIRIAM) + met.annotation["sbo"] = _SBO_METABOLITE + + if "rxn" in types: + rxns = _read_tsv(model_dir / "reactions.tsv").set_index("rxns") + for rxn in model.reactions: + if rxn.id in rxns.index: + _apply_row(rxn.annotation, rxns.loc[rxn.id], _RXN_ID2MIRIAM) + _assign_sbo(model) + + if "gene" in types: + genes = _read_tsv(model_dir / "genes.tsv").set_index("genes") + for gene in model.genes: + if gene.id in genes.index: + row = genes.loc[gene.id].copy() + row["genes"] = gene.id # the gene id itself is an ensembl id + _apply_row(gene.annotation, row, _GENE_ID2MIRIAM) + gene.annotation["sbo"] = _SBO_GENE + + return model + + +def _main() -> int: + from raven_toolbox.io import read_yaml_model + + repo_root = Path(__file__).resolve().parents[1] + model_dir = repo_root / "model" + model = read_yaml_model(model_dir / "Human-GEM.yml") + annotate_gem(model, model_dir) + n_rxn = sum(1 for r in model.reactions if any(k != "sbo" for k in r.annotation)) + n_met = sum(1 for m in model.metabolites if m.annotation) + n_gene = sum(1 for g in model.genes if g.annotation) + print(f"annotated reactions (cross-refs): {n_rxn}/{len(model.reactions)}") + print(f"annotated metabolites: {n_met}/{len(model.metabolites)}") + print(f"annotated genes: {n_gene}/{len(model.genes)}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(_main()) diff --git a/code/io/increaseHumanGEMVersion.py b/code/io/increaseHumanGEMVersion.py new file mode 100644 index 00000000..8ea29661 --- /dev/null +++ b/code/io/increaseHumanGEMVersion.py @@ -0,0 +1,211 @@ +"""Cut a new Human-GEM release: bump the version and regenerate the model exports. + +Python/raven-toolbox port of ``code/io/increaseHumanGEMVersion.m``. + +Reads ``model/Human-GEM.yml``, checks it against the annotation tables, then +regenerates every export in ``model/``: + +* ``Human-GEM.yml`` and ``Human-GEM.mat`` from the plain model (cross-references + stay in the TSV tables); +* ``Human-GEM.xml`` (SBML), ``Human-GEM.xlsx`` and ``Human-GEM.txt`` from a copy + that has the TSV cross-references and SBO terms merged in (see annotateGEM.py). + +Outside test mode it also refuses to run off ``main``, bumps ``version.txt`` and +fills the ``{{nRXN}}`` / ``{{nMET}}`` / ``{{nGENE}}`` / ``{{DATE}}`` placeholders +in ``README.md``. + +Usage: + python code/io/increaseHumanGEMVersion.py {major|minor|patch} + python code/io/increaseHumanGEMVersion.py patch --test # regenerate only +""" +from __future__ import annotations + +import argparse +import datetime +import platform +import subprocess +import sys +import warnings +from importlib import metadata as _md +from pathlib import Path + +import cobra +import pandas as pd + +REPO_ROOT = Path(__file__).resolve().parents[2] +MODEL_DIR = REPO_ROOT / "model" + +# Make the sibling code/annotateGEM.py importable regardless of the caller's cwd. +sys.path.insert(0, str(REPO_ROOT / "code")) +from annotateGEM import annotate_gem # noqa: E402 + +from raven_toolbox.io import read_yaml_model, write_yaml_model # noqa: E402 +from raven_toolbox.io.excel import _equation, export_to_excel # noqa: E402 + +# model attribute <-> TSV file <-> id column, for the consistency check. +_ID_TABLES = ( + ("reactions", "reactions.tsv", "rxns"), + ("metabolites", "metabolites.tsv", "mets"), + ("genes", "genes.tsv", "genes"), +) + + +def _bump(old: str, bump_type: str) -> str: + """Return the ``major``/``minor``/``patch`` increment of a ``x.y.z`` string.""" + parts = [int(p) for p in old.strip().split(".")] + if len(parts) != 3: + raise ValueError(f"version.txt is not x.y.z: {old!r}") + major, minor, patch = parts + if bump_type == "major": + major, minor, patch = major + 1, 0, 0 + elif bump_type == "minor": + minor, patch = minor + 1, 0 + elif bump_type == "patch": + patch += 1 + else: + raise ValueError('bump_type must be "major", "minor" or "patch"') + return f"{major}.{minor}.{patch}" + + +def _current_branch() -> str: + out = subprocess.run( + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + cwd=REPO_ROOT, capture_output=True, text=True, check=True, + ) + return out.stdout.strip() + + +def _check_tsv_consistency(model: cobra.Model) -> None: + """Error if any model id is missing from its TSV table, or vice versa.""" + problems = [] + for attr, fname, col in _ID_TABLES: + table = pd.read_csv(MODEL_DIR / fname, sep="\t", dtype=str, keep_default_na=False) + tsv_ids = set(table[col]) + model_ids = {entity.id for entity in getattr(model, attr)} + only_model = sorted(model_ids - tsv_ids) + only_tsv = sorted(tsv_ids - model_ids) + if only_model: + problems.append(f"in model.{attr} but not {fname}: {only_model}") + if only_tsv: + problems.append(f"in {fname} but not model.{attr}: {only_tsv}") + if problems: + raise ValueError("Model / TSV mismatch:\n " + "\n ".join(problems)) + + +def _set_version(model: cobra.Model, new_version: str) -> None: + """Write the version into the metaData block that write_yaml_model emits.""" + notes = model.notes or {} + meta = dict(notes.get("metaData") or {}) + meta["version"] = new_version # metaData wins in write_yaml_model + notes["metaData"] = meta + notes["version"] = new_version + model.notes = notes + + +def _write_txt(model: cobra.Model, path: Path) -> None: + """Single-file reaction table (RAVEN exportForGit txt / raven-toolbox layout).""" + with open(path, "w", encoding="utf-8") as fh: + fh.write("Rxn name\tFormula\tGene-reaction association\tLB\tUB\tObjective\n") + for r in model.reactions: + fh.write( + f"{r.id}\t{_equation(r)}\t{r.gene_reaction_rule}\t" + f"{r.lower_bound:g}\t{r.upper_bound:g}\t{r.objective_coefficient:g}\n" + ) + + +def _version(package: str) -> str: + try: + return _md.version(package) + except _md.PackageNotFoundError: + return "unknown" + + +def _write_dependencies(path: Path) -> None: + with open(path, "w", encoding="utf-8") as fh: + fh.write(f"python\t{platform.python_version()}\n") + fh.write(f"cobra\t{_version('cobra')}\n") + fh.write(f"raven_toolbox\t{_version('raven_toolbox')}\n") + + +def _export_annotated(model: cobra.Model) -> None: + """Write the annotated exports (xml / xlsx / txt) plus dependencies.txt.""" + annotated = annotate_gem(model.copy(), MODEL_DIR) + cobra.io.write_sbml_model(annotated, str(MODEL_DIR / "Human-GEM.xml")) + try: + export_to_excel(annotated, MODEL_DIR / "Human-GEM.xlsx") + except ImportError as exc: + warnings.warn( + f"Skipped Human-GEM.xlsx: {exc}. Install openpyxl before a real release.", + stacklevel=2, + ) + _write_txt(annotated, MODEL_DIR / "Human-GEM.txt") + _write_dependencies(MODEL_DIR / "dependencies.txt") + + +def _update_readme(model: cobra.Model) -> None: + readme = REPO_ROOT / "README.md" + content = readme.read_text(encoding="utf-8") + today = datetime.date.today().isoformat() + for token, value in ( + ("{{DATE}}", today), + ("{{nRXN}}", str(len(model.reactions))), + ("{{nMET}}", str(len(model.metabolites))), + ("{{nGENE}}", str(len(model.genes))), + ): + content = content.replace(token, value) + readme.write_text(content, encoding="utf-8") + + +def increase_human_gem_version(bump_type: str, test: bool = False) -> str | None: + """Regenerate the model exports and (unless ``test``) bump the version. + + Returns the new version string, or ``None`` in test mode. + """ + version_file = REPO_ROOT / "version.txt" + new_version = None + + if not test: + branch = _current_branch() + if branch != "main": + raise RuntimeError(f"not on main (current branch: {branch})") + new_version = _bump(version_file.read_text(encoding="utf-8"), bump_type) + + model = read_yaml_model(MODEL_DIR / "Human-GEM.yml") + + if not test: + _set_version(model, new_version) + + _check_tsv_consistency(model) + + # Plain exports (cross-references live in the TSV tables, not here). + write_yaml_model(model, MODEL_DIR / "Human-GEM.yml") + cobra.io.save_matlab_model(model, str(MODEL_DIR / "Human-GEM.mat"), varname="humanGEM") + + # Annotated exports (TSV cross-references + SBO terms merged in). + _export_annotated(model) + + if not test: + version_file.write_text(new_version, encoding="utf-8") + _update_readme(model) + print(f"Human-GEM bumped to {new_version}") + else: + print("Test run: exports regenerated, version unchanged.") + + return new_version + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("bump_type", choices=("major", "minor", "patch"), + help="which part of the semantic version to increment") + parser.add_argument("--test", action="store_true", + help="regenerate the exports without checking the branch or " + "bumping the version (may run on a development branch)") + args = parser.parse_args(argv) + increase_human_gem_version(args.bump_type, test=args.test) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/code/test/annotateModel.py b/code/test/annotateModel.py deleted file mode 100644 index f44b4954..00000000 --- a/code/test/annotateModel.py +++ /dev/null @@ -1,127 +0,0 @@ -"""Attach database cross-references from the annotation tables to a cobra model. - -The committed YAML model carries only ids and names; the cross-references live in -model/metabolites.tsv, model/reactions.tsv and model/genes.tsv. MEMOTE reads -annotations from the SBML export, so without them it scores every annotation test -0% even though Human-GEM has the cross-references. This enriches an in-memory model -- used only to build the temporary SBML that MEMOTE runs on, never committed - so -the MEMOTE annotation scores reflect the identifiers Human-GEM actually carries. - -Only registry (identifiers.org) namespaces MEMOTE can validate are attached, and -values are normalised to each namespace's expected form (e.g. Rhea drops its -"RHEA:" prefix; KEGG metabolite ids are split into compound/glycan/drug by prefix). -Legacy-only columns (EHMN, HepatoNET1, Recon3D, HMR2, Ratcon) are skipped. - -Usage: - import annotateModel - annotateModel.enrich(model) # mutates the model in place -""" - -import csv -from pathlib import Path - -REPO_ROOT = Path(__file__).resolve().parents[2] -METABOLITES_TSV = REPO_ROOT / "model" / "metabolites.tsv" -REACTIONS_TSV = REPO_ROOT / "model" / "reactions.tsv" -GENES_TSV = REPO_ROOT / "model" / "genes.tsv" - -# TSV column -> (identifiers.org namespace, prefix to strip so the value conforms). -MET_MAP = { - "metBiGGID": ("bigg.metabolite", None), - "metHMDBID": ("hmdb", None), - "metChEBIID": ("chebi", None), # values already "CHEBI:1234" - "metPubChemID": ("pubchem.compound", None), - "metLipidMapsID": ("lipidmaps", None), - "metMetaNetXID": ("metanetx.chemical", None), - "metSeedID": ("seed.compound", None), -} -RXN_MAP = { - "rxnKEGGID": ("kegg.reaction", None), - "rxnBiGGID": ("bigg.reaction", None), - "rxnMetaNetXID": ("metanetx.reaction", None), - "rxnRheaID": ("rhea", "RHEA:"), # identifiers.org rhea wants the bare number - "rxnREACTOMEID": ("reactome", None), - "rxnTCDBID": ("tcdb", None), -} -GENE_MAP = { - "geneUniProtID": ("uniprot", None), - "geneEntrezID": ("ncbigene", None), -} -# KEGG metabolite ids span three namespaces, told apart by their first letter. -KEGG_MET_NS = {"C": "kegg.compound", "G": "kegg.glycan", "D": "kegg.drug"} - - -def _read(path: Path) -> list[dict]: - with open(path, newline="", encoding="utf-8") as fh: - return list(csv.DictReader(fh, delimiter="\t")) - - -def _values(raw: str, strip_prefix) -> list[str]: - parts = [p.strip() for p in (raw or "").split(";") if p.strip()] - if strip_prefix: - parts = [p[len(strip_prefix):] if p.startswith(strip_prefix) else p for p in parts] - return parts - - -def _add(ann: dict, namespace: str, value: str) -> None: - """Add one identifier under a namespace, keeping a list when there are several.""" - cur = ann.get(namespace) - if cur is None: - ann[namespace] = value - elif isinstance(cur, list): - if value not in cur: - cur.append(value) - elif cur != value: - ann[namespace] = [cur, value] - - -def _apply(entity, row: dict, mapping: dict) -> bool: - ann = dict(entity.annotation or {}) - before = len(ann) - for column, (namespace, strip) in mapping.items(): - for value in _values(row.get(column, ""), strip): - _add(ann, namespace, value) - entity.annotation = ann - return len(ann) > before - - -def enrich(model) -> dict: - """Attach cross-references to every metabolite/reaction/gene that has a table - row. Returns {'metabolites': n, 'reactions': n, 'genes': n} annotated.""" - mets = {r["mets"]: r for r in _read(METABOLITES_TSV)} - rxns = {r["rxns"]: r for r in _read(REACTIONS_TSV)} - genes = {r["genes"]: r for r in _read(GENES_TSV)} - counts = {"metabolites": 0, "reactions": 0, "genes": 0} - - for met in model.metabolites: - row = mets.get(met.id) - if not row: - continue - touched = _apply(met, row, MET_MAP) - ann = dict(met.annotation or {}) - for value in _values(row.get("metKEGGID", ""), None): - namespace = KEGG_MET_NS.get(value[:1]) - if namespace: - _add(ann, namespace, value) - touched = True - met.annotation = ann - counts["metabolites"] += touched - - for rxn in model.reactions: - row = rxns.get(rxn.id) - if row and _apply(rxn, row, RXN_MAP): - counts["reactions"] += 1 - - for gene in model.genes: - ann = dict(gene.annotation or {}) - # The gene id is itself the Ensembl gene identifier. - if gene.id.startswith("ENSG"): - _add(ann, "ensembl", gene.id) - gene.annotation = ann - touched = bool(ann) - row = genes.get(gene.id) - if row: - touched = _apply(gene, row, GENE_MAP) or touched - counts["genes"] += touched - - return counts diff --git a/code/test/memoteSnapshot.py b/code/test/memoteSnapshot.py index e24d1e40..cd9c7671 100644 --- a/code/test/memoteSnapshot.py +++ b/code/test/memoteSnapshot.py @@ -21,10 +21,10 @@ only its own section, so a routine subset run never overwrites a committed full-suite score (see _write_section). -Before exporting the SBML, the model is enriched with the database cross-references -from the annotation tables (see annotateModel.py) so MEMOTE's annotation tests score -against the identifiers Human-GEM actually carries. The enriched model exists only in -memory for the temporary SBML; it is never committed. +Before exporting the SBML, the model is enriched with the cross-references and SBO +terms from the annotation tables (the canonical code/annotateGEM.py helper) so +MEMOTE's annotation tests score against the identifiers Human-GEM actually carries. +The enriched model exists only in memory for the temporary SBML; it is never committed. Set GRB_LICENSE_FILE (a full Gurobi licence) to run with Gurobi; the genome-scale MILPs are impractical with GLPK. Without it the script falls back to the default @@ -38,14 +38,18 @@ import os import sys import tempfile +from pathlib import Path import cobra import memote.suite.api as api from memote.suite.reporting import ReportConfiguration, SnapshotReport -import annotateModel +# annotateGEM lives in code/ (one level up), the canonical annotation helper. +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from annotateGEM import annotate_gem MODEL_FILE = "model/Human-GEM.yml" +MODEL_DIR = "model" # holds the reactions/metabolites/genes TSV tables RESULT_JSON = "memote_result.json" # repo root -> uploaded as artifact, not committed SCORE_MD = "data/testResults/memote_score.md" @@ -214,13 +218,16 @@ def main() -> int: # temporary SBML file first (memote fails on a .yml directly). model = cobra.io.load_yaml_model(MODEL_FILE) - # The YAML model has only ids and names; attach the database cross-references - # from the annotation tables so MEMOTE's annotation tests see them. This mutates - # the in-memory model only - the SBML written below is temporary and the enriched - # model is never committed. - counts = annotateModel.enrich(model) - print(f"Annotated for MEMOTE (temporary): {counts['metabolites']} metabolites, " - f"{counts['reactions']} reactions, {counts['genes']} genes.", flush=True) + # The YAML model has only ids and names; attach the cross-references and SBO + # terms from the annotation tables (the canonical annotateGEM helper) so MEMOTE's + # annotation tests see them. This mutates the in-memory model only - the SBML + # written below is temporary and the enriched model is never committed. + annotate_gem(model, MODEL_DIR) + n_met = sum(1 for m in model.metabolites if any(k != "sbo" for k in m.annotation)) + n_rxn = sum(1 for r in model.reactions if any(k != "sbo" for k in r.annotation)) + n_gene = sum(1 for g in model.genes if any(k != "sbo" for k in g.annotation)) + print(f"Annotated for MEMOTE (temporary): {n_met} metabolites, {n_rxn} reactions, " + f"{n_gene} genes cross-referenced, plus SBO terms.", flush=True) sbml_path = os.path.join(tempfile.gettempdir(), "human-gem.xml") cobra.io.write_sbml_model(model, sbml_path) diff --git a/data/testResults/README.md b/data/testResults/README.md index a571e8d0..96b77f18 100644 --- a/data/testResults/README.md +++ b/data/testResults/README.md @@ -168,11 +168,12 @@ stoichiometric-consistency MILP and matrix-rank tests that dominate runtime). Comment `/run memote` to run the full suite; the score then updates in place. Higher is better, so the comment warns only when the score drops versus the target branch. -Before running, the model is enriched with the database cross-references from the -annotation tables (`annotateModel.py`), so the annotation tests score against the -identifiers Human-GEM actually carries rather than the bare ids in the YAML. The -enriched model is used only to build the temporary SBML MEMOTE reads; it is not -committed. The score is stored in two sections, `Core subset` and `Full suite`. +Before running, the model is enriched with the cross-references and SBO terms from +the annotation tables (the canonical `code/annotateGEM.py` helper), so the annotation +tests score against the identifiers Human-GEM actually carries rather than the bare +ids in the YAML. The enriched model is used only to build the temporary SBML MEMOTE +reads; it is not committed. The score is stored in two sections, `Core subset` and +`Full suite`. ### Gene essentiality (Hart 2015) Gene-essentiality predictions in five cell-line-specific GEMs (DLD1, GBM, HCT116, From 6a57fb4a45d4c2c2ae8a540bd008bede4e8ef3d6 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Wed, 15 Jul 2026 23:33:51 +0200 Subject: [PATCH 06/11] ci: collapse model-QC back to a single results commit The MEMOTE fast subset finishes quickly, so the interim fast-checks commit and comment update are unnecessary. Run every check, commit once at the end, and post the comment from the committed files - keeping the invariant that the comment never shows numbers or CSV links that are not yet on the branch. --- .github/workflows/model-qc.yml | 81 +++++++++------------------------- 1 file changed, 20 insertions(+), 61 deletions(-) diff --git a/.github/workflows/model-qc.yml b/.github/workflows/model-qc.yml index 30b37e53..08b92ea6 100644 --- a/.github/workflows/model-qc.yml +++ b/.github/workflows/model-qc.yml @@ -15,12 +15,11 @@ env: qc_structure_consistency.csv # One job runs every check and edits a single pull-request comment. It posts a -# "running" comment immediately (all rows as hourglasses, no numbers), then works in -# two phases: it commits the fast-check results and updates the comment from the -# committed files (MEMOTE still a running row), then commits the MEMOTE score and -# updates the comment again. Each commit lands before its comment update, so the -# comment never reports numbers, or links to CSVs, that are not yet on the branch. -# Commits use [skip ci] and only happen if something changed. +# "running" comment immediately (all rows as hourglasses, no numbers), runs every +# check including the MEMOTE fast subset, commits the results once at the end (with +# [skip ci], and only if something changed), and only then edits the comment to show +# the results - so the comment never reports numbers, or links to CSVs, that are not +# yet on the branch. jobs: qc: runs-on: ubuntu-latest @@ -133,57 +132,9 @@ jobs: continue-on-error: true run: python code/test/testMetabolicTasks.py all - # Fast checks are in; MEMOTE is still running. Commit the fast results first, - # then update the comment from the committed files - so every number shown is - # already on the branch. MEMOTE stays a running row until its own commit below. - - name: Render fast-check report - uses: ./.github/actions/post-qc-comment - with: - mode: build - running-groups: memote - base-ref: ${{ env.BASE_REF }} - base-dir: ${{ env.BASE_DIR }} - results-url-base: ${{ env.RESULTS_URL_BASE }} - run-url: ${{ env.RUN_URL }} - github-token: ${{ secrets.GITHUB_TOKEN }} - - - name: Mention PR# in README.md - env: - PR_NUMBER: ${{ github.event.number }} - run: | - for tag in "model QC" "MEMOTE" "MACAW"; do - sed -i -e "s/[[:digit:]]\{3,4\}\*\* ($tag/$PR_NUMBER\*\* ($tag/" data/testResults/README.md - done - - - name: Update local branch before committing (fast checks) - env: - BRANCH_NAME: ${{ github.head_ref || github.ref_name }} - run: | - git stash - git fetch - git checkout $BRANCH_NAME - if git stash list | grep -q 'stash@{'; then - git stash pop - fi - - - name: Commit fast-check results - uses: stefanzweifel/git-auto-commit-action@v7 - with: - commit_user_name: memote-bot - # [skip ci] so this results commit does not re-trigger the workflow. - commit_message: "chore: update model QC results (fast checks) [skip ci]" - file_pattern: data/testResults/* - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - name: Update comment with fast checks - uses: ./.github/actions/post-qc-comment - with: - mode: post - run-url: ${{ env.RUN_URL }} - base-ref: ${{ env.BASE_REF }} - github-token: ${{ secrets.GITHUB_TOKEN }} - + # The MEMOTE fast subset finishes quickly, so there is no interim update: all + # checks (fast + MEMOTE) run, then everything is committed once and the comment + # is posted from the committed files below. Until then the running comment stands. - name: Install MEMOTE dependencies run: pip install memote gurobipy @@ -211,9 +162,9 @@ jobs: timeout 2400 python code/test/memoteSnapshot.py \ || echo "::warning::MEMOTE did not finish within 2400s; score unavailable this run." - # MEMOTE is in: render the final summary so it is part of the commit below, then - # post from the committed file. As with the fast phase, the comment is updated - # only after the results are committed (so its numbers and CSV links resolve). + # Everything is in: render the final summary so it is part of the commit below, + # then post from the committed file - the comment is updated only after the + # results are committed (so its numbers and CSV links resolve). - name: Render final report uses: ./.github/actions/post-qc-comment with: @@ -225,7 +176,15 @@ jobs: run-url: ${{ env.RUN_URL }} github-token: ${{ secrets.GITHUB_TOKEN }} - - name: Update local branch before committing (final) + - name: Mention PR# in README.md + env: + PR_NUMBER: ${{ github.event.number }} + run: | + for tag in "model QC" "MEMOTE" "MACAW"; do + sed -i -e "s/[[:digit:]]\{3,4\}\*\* ($tag/$PR_NUMBER\*\* ($tag/" data/testResults/README.md + done + + - name: Update local branch before committing changes env: BRANCH_NAME: ${{ github.head_ref || github.ref_name }} run: | From af1932386ad14bf08c1bc7bfce2e73579393ebab Mon Sep 17 00:00:00 2001 From: edkerk <7326655+edkerk@users.noreply.github.com> Date: Wed, 15 Jul 2026 21:47:00 +0000 Subject: [PATCH 07/11] chore: update model QC results (fast checks) [skip ci] --- data/testResults/model_qc_summary.md | 46 +--------------------------- 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/data/testResults/model_qc_summary.md b/data/testResults/model_qc_summary.md index a18aa0b1..9f3c7613 100644 --- a/data/testResults/model_qc_summary.md +++ b/data/testResults/model_qc_summary.md @@ -45,51 +45,7 @@ _Duplicate keys (model unloadable) and no growth block the merge; every other ro ### [MEMOTE](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#memote) -**Total score: 20.2%** (core subset)   0 - -| Section | Score | Δ vs base | -| --- | ---: | ---: | -| consistency | 42.4% | 0 | -| annotation_met | 25.0% | 0 | -| annotation_rxn | 25.0% | 0 | -| annotation_gene | 0.0% | 0 | -| annotation_sbo | 0.0% | 0 | - -
Per-test scores - -| Section | Test | Score | -| --- | --- | ---: | -| Consistency | Stoichiometric Consistency | 100.0% | -| Consistency | Mass Balance | 0.8% | -| Consistency | Charge Balance | 2.1% | -| Consistency | Metabolite Connectivity | 0.0% | -| Consistency | Unbounded Flux In Default Medium | 100.0% | -| Annotation - Metabolites | Presence of Metabolite Annotation | 100.0% | -| Annotation - Metabolites | Metabolite Annotations Per Database | 100.0% | -| Annotation - Metabolites | Metabolite Annotation Conformity Per Database | 100.0% | -| Annotation - Metabolites | Uniform Metabolite Identifier Namespace | 0.0% | -| Annotation - Reactions | Presence of Reaction Annotation | 100.0% | -| Annotation - Reactions | Reaction Annotations Per Database | 100.0% | -| Annotation - Reactions | Reaction Annotation Conformity Per Database | 100.0% | -| Annotation - Reactions | Uniform Reaction Identifier Namespace | 0.0% | -| Annotation - Genes | Presence of Gene Annotation | 100.0% | -| Annotation - Genes | Gene Annotations Per Database | 100.0% | -| Annotation - Genes | Gene Annotation Conformity Per Database | 100.0% | -| Annotation - SBO Terms | Metabolite General SBO Presence | 100.0% | -| Annotation - SBO Terms | Metabolite SBO:0000247 Presence | 100.0% | -| Annotation - SBO Terms | Reaction General SBO Presence | 100.0% | -| Annotation - SBO Terms | Metabolic Reaction SBO:0000176 Presence | 100.0% | -| Annotation - SBO Terms | Transport Reaction SBO:0000185 Presence | 100.0% | -| Annotation - SBO Terms | Exchange Reaction SBO:0000627 Presence | 100.0% | -| Annotation - SBO Terms | Demand Reaction SBO:0000628 Presence | 100.0% | -| Annotation - SBO Terms | Sink Reactions SBO:0000632 Presence | 100.0% | -| Annotation - SBO Terms | Gene General SBO Presence | 100.0% | -| Annotation - SBO Terms | Gene SBO:0000243 Presence | 100.0% | -| Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 100.0% | - -
- -_Full suite not run for this commit; comment_ `/run memote` _to add it._ +_running_ · :hourglass_flowing_sand: _The score above is the fast core subset. Comment_ `/run memote` _to run the full suite on this pull request; the score updates here when it finishes._ From 6d0e24e30adb6dc7267994ee716e5b015a56eae2 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Wed, 15 Jul 2026 23:47:44 +0200 Subject: [PATCH 08/11] refactor: use raven-toolbox for SBO terms and model I/O annotateGEM now delegates metabolite and reaction SBO assignment to the canonical raven_toolbox.annotation.add_sbo_terms (passing Human-GEM's biomass reaction name) instead of a hand-rolled version; it keeps the Human-GEM-specific TSV cross-reference merge, and still sets the gene SBO term (SBO:0000243) that add_sbo_terms does not cover. memoteSnapshot loads the model with raven_toolbox.io.read_yaml_model, like the other RAVEN-based tests. increaseHumanGEMVersion writes its exports with raven_toolbox.io.export_for_git (yml/ mat plain, xml/xlsx/txt annotated) instead of calling cobra's writers and hand-rolled txt / dependencies writers directly. Validated on the full model: SBO terms assigned to all metabolites/reactions/genes (biomass MAR13082 -> SBO:0000629), cross-references merged without clobbering existing ones, and export_for_git writes the annotated SBML. --- code/annotateGEM.py | 92 ++++++------------------------ code/io/increaseHumanGEMVersion.py | 60 +++---------------- code/test/memoteSnapshot.py | 6 +- 3 files changed, 32 insertions(+), 126 deletions(-) diff --git a/code/annotateGEM.py b/code/annotateGEM.py index 38c1154d..c8040395 100644 --- a/code/annotateGEM.py +++ b/code/annotateGEM.py @@ -6,12 +6,13 @@ ``smiles``); the full set of external identifiers lives in the annotation tables ``model/reactions.tsv``, ``model/metabolites.tsv`` and ``model/genes.tsv``. This module reads those tables and writes the identifiers -onto each cobra entity's ``annotation`` dict (namespace -> list of ids), and -assigns an SBO term to every reaction (classified into -biochemical/transport/exchange/demand/sink/biomass), metabolite (simple chemical) -and gene. The exported SBML / Excel / txt then carry the annotation, while the YAML -and ``.mat`` files stay annotation-light (their cross-references remain the TSV -tables), exactly as the MATLAB release flow produces them. +onto each cobra entity's ``annotation`` dict (namespace -> list of ids). SBO terms +for metabolites and reactions come from raven_toolbox's canonical ``add_sbo_terms`` +(classifying exchange/demand/sink, transport, biomass, simple chemical, ...); genes, +which that helper does not cover, get SBO:0000243 here. The exported SBML / Excel / +txt then carry the annotation, while the YAML and ``.mat`` files stay +annotation-light (their cross-references remain the TSV tables), exactly as the +MATLAB release flow produces them. Used by ``code/io/increaseHumanGEMVersion.py``; can also be run standalone to inspect the merge: @@ -24,6 +25,7 @@ import cobra import pandas as pd +from raven_toolbox.annotation import add_sbo_terms # Map TSV column names to identifiers.org namespaces (from annotateGEM.m id2miriam). _RXN_ID2MIRIAM = { @@ -56,19 +58,11 @@ "geneEntrezID": "ncbigene", } -# Reaction SBO terms (annotateGEM.m). Precedence low -> high: default, biomass, -# transport, then boundary (a later match overrides an earlier one). Boundary -# reactions are split into exchange/demand/sink, matching how cobra (and hence -# MEMOTE) classifies them, so each gets the SBO term its check expects. -_SBO_DEFAULT = "SBO:0000176" # biochemical reaction -_SBO_BIOMASS = "SBO:0000629" # biomass production -_SBO_TRANSPORT = "SBO:0000185" # translocation reaction -_SBO_EXCHANGE = "SBO:0000627" # exchange reaction -_SBO_DEMAND = "SBO:0000628" # demand reaction -_SBO_SINK = "SBO:0000632" # sink reaction -# Metabolite and gene SBO terms (one each; no classification needed). -_SBO_METABOLITE = "SBO:0000247" # simple chemical -_SBO_GENE = "SBO:0000243" # gene +# Reaction and metabolite SBO terms come from raven_toolbox.annotation.add_sbo_terms +# (the canonical assignment); it does not cover genes, so gene SBO is set here. +_SBO_GENE = "SBO:0000243" # gene +# Human-GEM's biomass (objective) reaction, so add_sbo_terms tags it SBO:0000629. +_BIOMASS_RXN_NAME = "Generic human cell biomass reaction" def _read_tsv(path: Path) -> pd.DataFrame: @@ -112,58 +106,6 @@ def _apply_row(annotation: dict, row: pd.Series, id2miriam: dict) -> None: annotation[namespace] = list(dict.fromkeys(merged)) -def _is_transport(rxn: cobra.Reaction) -> bool: - """True if a metabolite name appears in more than one compartment (RAVEN - getTransportRxns): the reaction moves a species across compartments.""" - comps_by_name: dict[str, set[str]] = {} - for met in rxn.metabolites: - comps_by_name.setdefault(met.name, set()).add(met.compartment) - return any(len(comps) > 1 for comps in comps_by_name.values()) - - -def _boundary_sbo(model: cobra.Model) -> dict: - """Map each boundary reaction to its SBO term (exchange / demand / sink). - - Uses cobra's own ``exchanges`` / ``demands`` / ``sinks`` classification, which - MEMOTE also uses, so the assigned term matches the check MEMOTE will apply. If - cobra cannot classify (e.g. it fails to find an external compartment, or the - model type lacks these properties), fall back to treating every boundary - reaction as an exchange, as the original port did.""" - try: - exchanges, demands, sinks = model.exchanges, model.demands, model.sinks - except Exception: # noqa: BLE001 - any classification failure -> safe fallback - exchanges, demands, sinks = model.boundary, [], [] - sbo = {} - for rxn in exchanges: - sbo[rxn.id] = _SBO_EXCHANGE - for rxn in demands: - sbo[rxn.id] = _SBO_DEMAND - for rxn in sinks: - sbo[rxn.id] = _SBO_SINK - # Any boundary reaction cobra did not place lands as an exchange. - for rxn in model.boundary: - sbo.setdefault(rxn.id, _SBO_EXCHANGE) - return sbo - - -def _assign_sbo(model: cobra.Model) -> None: - """Set ``annotation['sbo']`` on every reaction (annotateGEM.m SBO logic).""" - boundary_sbo = _boundary_sbo(model) - for rxn in model.reactions: - sbo = _SBO_DEFAULT - is_biomass = "biomass" in rxn.id.lower() or any( - met.name.lower() == "biomass" and coeff > 0 - for met, coeff in rxn.metabolites.items() - ) - if is_biomass: - sbo = _SBO_BIOMASS - if _is_transport(rxn): - sbo = _SBO_TRANSPORT - if rxn.id in boundary_sbo: - sbo = boundary_sbo[rxn.id] - rxn.annotation["sbo"] = sbo - - def annotate_gem( model: cobra.Model, model_dir: str | Path, @@ -195,14 +137,12 @@ def annotate_gem( for met in model.metabolites: if met.id in mets.index: _apply_row(met.annotation, mets.loc[met.id], _MET_ID2MIRIAM) - met.annotation["sbo"] = _SBO_METABOLITE if "rxn" in types: rxns = _read_tsv(model_dir / "reactions.tsv").set_index("rxns") for rxn in model.reactions: if rxn.id in rxns.index: _apply_row(rxn.annotation, rxns.loc[rxn.id], _RXN_ID2MIRIAM) - _assign_sbo(model) if "gene" in types: genes = _read_tsv(model_dir / "genes.tsv").set_index("genes") @@ -211,8 +151,14 @@ def annotate_gem( row = genes.loc[gene.id].copy() row["genes"] = gene.id # the gene id itself is an ensembl id _apply_row(gene.annotation, row, _GENE_ID2MIRIAM) + # add_sbo_terms below covers metabolites and reactions, not genes. gene.annotation["sbo"] = _SBO_GENE + # SBO terms for metabolites and reactions: the canonical raven-toolbox + # assignment (exchange/demand/sink, transport, biomass, simple chemical, ...). + if "met" in types or "rxn" in types: + add_sbo_terms(model, biomass_rxn_name=_BIOMASS_RXN_NAME) + return model diff --git a/code/io/increaseHumanGEMVersion.py b/code/io/increaseHumanGEMVersion.py index 8ea29661..568275b2 100644 --- a/code/io/increaseHumanGEMVersion.py +++ b/code/io/increaseHumanGEMVersion.py @@ -22,11 +22,8 @@ import argparse import datetime -import platform import subprocess import sys -import warnings -from importlib import metadata as _md from pathlib import Path import cobra @@ -39,8 +36,7 @@ sys.path.insert(0, str(REPO_ROOT / "code")) from annotateGEM import annotate_gem # noqa: E402 -from raven_toolbox.io import read_yaml_model, write_yaml_model # noqa: E402 -from raven_toolbox.io.excel import _equation, export_to_excel # noqa: E402 +from raven_toolbox.io import export_for_git, read_yaml_model # noqa: E402 # model attribute <-> TSV file <-> id column, for the consistency check. _ID_TABLES = ( @@ -102,46 +98,6 @@ def _set_version(model: cobra.Model, new_version: str) -> None: model.notes = notes -def _write_txt(model: cobra.Model, path: Path) -> None: - """Single-file reaction table (RAVEN exportForGit txt / raven-toolbox layout).""" - with open(path, "w", encoding="utf-8") as fh: - fh.write("Rxn name\tFormula\tGene-reaction association\tLB\tUB\tObjective\n") - for r in model.reactions: - fh.write( - f"{r.id}\t{_equation(r)}\t{r.gene_reaction_rule}\t" - f"{r.lower_bound:g}\t{r.upper_bound:g}\t{r.objective_coefficient:g}\n" - ) - - -def _version(package: str) -> str: - try: - return _md.version(package) - except _md.PackageNotFoundError: - return "unknown" - - -def _write_dependencies(path: Path) -> None: - with open(path, "w", encoding="utf-8") as fh: - fh.write(f"python\t{platform.python_version()}\n") - fh.write(f"cobra\t{_version('cobra')}\n") - fh.write(f"raven_toolbox\t{_version('raven_toolbox')}\n") - - -def _export_annotated(model: cobra.Model) -> None: - """Write the annotated exports (xml / xlsx / txt) plus dependencies.txt.""" - annotated = annotate_gem(model.copy(), MODEL_DIR) - cobra.io.write_sbml_model(annotated, str(MODEL_DIR / "Human-GEM.xml")) - try: - export_to_excel(annotated, MODEL_DIR / "Human-GEM.xlsx") - except ImportError as exc: - warnings.warn( - f"Skipped Human-GEM.xlsx: {exc}. Install openpyxl before a real release.", - stacklevel=2, - ) - _write_txt(annotated, MODEL_DIR / "Human-GEM.txt") - _write_dependencies(MODEL_DIR / "dependencies.txt") - - def _update_readme(model: cobra.Model) -> None: readme = REPO_ROOT / "README.md" content = readme.read_text(encoding="utf-8") @@ -177,12 +133,14 @@ def increase_human_gem_version(bump_type: str, test: bool = False) -> str | None _check_tsv_consistency(model) - # Plain exports (cross-references live in the TSV tables, not here). - write_yaml_model(model, MODEL_DIR / "Human-GEM.yml") - cobra.io.save_matlab_model(model, str(MODEL_DIR / "Human-GEM.mat"), varname="humanGEM") - - # Annotated exports (TSV cross-references + SBO terms merged in). - _export_annotated(model) + # Export via raven-toolbox's Standard-GEM writer. The plain formats (yml/mat) + # keep their cross-references in the TSV tables; the annotated formats (xml/xlsx/ + # txt) carry the merged TSV cross-references and SBO terms (see annotateGEM.py). + # export_for_git also (re)writes model/dependencies.txt. + export_for_git(model, MODEL_DIR, prefix="Human-GEM", + formats=("yml", "mat"), sub_dirs=False) + export_for_git(annotate_gem(model.copy(), MODEL_DIR), MODEL_DIR, + prefix="Human-GEM", formats=("xml", "xlsx", "txt"), sub_dirs=False) if not test: version_file.write_text(new_version, encoding="utf-8") diff --git a/code/test/memoteSnapshot.py b/code/test/memoteSnapshot.py index cd9c7671..32b3d815 100644 --- a/code/test/memoteSnapshot.py +++ b/code/test/memoteSnapshot.py @@ -47,6 +47,7 @@ # annotateGEM lives in code/ (one level up), the canonical annotation helper. sys.path.insert(0, str(Path(__file__).resolve().parents[1])) from annotateGEM import annotate_gem +from raven_toolbox.io import read_yaml_model MODEL_FILE = "model/Human-GEM.yml" MODEL_DIR = "model" # holds the reactions/metabolites/genes TSV tables @@ -215,8 +216,9 @@ def main() -> int: cobra.Configuration().solver = "gurobi" # memote reads an SBML model, so convert the canonical YAML model to a - # temporary SBML file first (memote fails on a .yml directly). - model = cobra.io.load_yaml_model(MODEL_FILE) + # temporary SBML file first (memote fails on a .yml directly). Load via + # raven-toolbox, like the other RAVEN-based tests. + model = read_yaml_model(MODEL_FILE) # The YAML model has only ids and names; attach the cross-references and SBO # terms from the annotation tables (the canonical annotateGEM helper) so MEMOTE's From ebab5eca4684e324c5325d7316bc4d3553243b7a Mon Sep 17 00:00:00 2001 From: edkerk <7326655+edkerk@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:06:29 +0000 Subject: [PATCH 09/11] chore: update model QC results [skip ci] --- data/testResults/memote_score.md | 46 ++++++++++++++-------------- data/testResults/model_qc_summary.md | 46 +++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/data/testResults/memote_score.md b/data/testResults/memote_score.md index a75708af..7d554e37 100644 --- a/data/testResults/memote_score.md +++ b/data/testResults/memote_score.md @@ -5,17 +5,17 @@ Mode: core subset. Skipped (slow) tests: test_stoichiometric_consistency, test_unconserved_metabolites, test_inconsistent_min_stoichiometry, test_detect_energy_generating_cycles, test_find_stoichiometrically_balanced_cycles, test_blocked_reactions, test_find_reactions_unbounded_flux_default_condition, test_find_metabolites_not_produced_with_open_bounds, test_find_metabolites_not_consumed_with_open_bounds, test_number_independent_conservation_relations, test_matrix_rank, test_degrees_of_freedom. -**Total score: 20.2%** +**Total score: 63.2%** ### Section scores | Section | Score | | --- | ---: | | consistency | 42.4% | -| annotation_met | 25.0% | -| annotation_rxn | 25.0% | -| annotation_gene | 0.0% | -| annotation_sbo | 0.0% | +| annotation_met | 73.0% | +| annotation_rxn | 72.7% | +| annotation_gene | 46.7% | +| annotation_sbo | 81.7% | ### Detailed scores @@ -26,28 +26,28 @@ Skipped (slow) tests: test_stoichiometric_consistency, test_unconserved_metaboli | Consistency | Charge Balance | 2.1% | | Consistency | Metabolite Connectivity | 0.0% | | Consistency | Unbounded Flux In Default Medium | 100.0% | -| Annotation - Metabolites | Presence of Metabolite Annotation | 100.0% | -| Annotation - Metabolites | Metabolite Annotations Per Database | 100.0% | -| Annotation - Metabolites | Metabolite Annotation Conformity Per Database | 100.0% | +| Annotation - Metabolites | Presence of Metabolite Annotation | 0.0% | +| Annotation - Metabolites | Metabolite Annotations Per Database | 62.3% | +| Annotation - Metabolites | Metabolite Annotation Conformity Per Database | 45.8% | | Annotation - Metabolites | Uniform Metabolite Identifier Namespace | 0.0% | -| Annotation - Reactions | Presence of Reaction Annotation | 100.0% | -| Annotation - Reactions | Reaction Annotations Per Database | 100.0% | -| Annotation - Reactions | Reaction Annotation Conformity Per Database | 100.0% | +| Annotation - Reactions | Presence of Reaction Annotation | 0.0% | +| Annotation - Reactions | Reaction Annotations Per Database | 75.9% | +| Annotation - Reactions | Reaction Annotation Conformity Per Database | 33.3% | | Annotation - Reactions | Uniform Reaction Identifier Namespace | 0.0% | -| Annotation - Genes | Presence of Gene Annotation | 100.0% | -| Annotation - Genes | Gene Annotations Per Database | 100.0% | -| Annotation - Genes | Gene Annotation Conformity Per Database | 100.0% | -| Annotation - SBO Terms | Metabolite General SBO Presence | 100.0% | -| Annotation - SBO Terms | Metabolite SBO:0000247 Presence | 100.0% | -| Annotation - SBO Terms | Reaction General SBO Presence | 100.0% | -| Annotation - SBO Terms | Metabolic Reaction SBO:0000176 Presence | 100.0% | -| Annotation - SBO Terms | Transport Reaction SBO:0000185 Presence | 100.0% | -| Annotation - SBO Terms | Exchange Reaction SBO:0000627 Presence | 100.0% | +| Annotation - Genes | Presence of Gene Annotation | 0.0% | +| Annotation - Genes | Gene Annotations Per Database | 80.0% | +| Annotation - Genes | Gene Annotation Conformity Per Database | 80.0% | +| Annotation - SBO Terms | Metabolite General SBO Presence | 0.0% | +| Annotation - SBO Terms | Metabolite SBO:0000247 Presence | 0.1% | +| Annotation - SBO Terms | Reaction General SBO Presence | 0.0% | +| Annotation - SBO Terms | Metabolic Reaction SBO:0000176 Presence | 0.0% | +| Annotation - SBO Terms | Transport Reaction SBO:0000185 Presence | 0.7% | +| Annotation - SBO Terms | Exchange Reaction SBO:0000627 Presence | 0.0% | | Annotation - SBO Terms | Demand Reaction SBO:0000628 Presence | 100.0% | | Annotation - SBO Terms | Sink Reactions SBO:0000632 Presence | 100.0% | -| Annotation - SBO Terms | Gene General SBO Presence | 100.0% | -| Annotation - SBO Terms | Gene SBO:0000243 Presence | 100.0% | -| Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 100.0% | +| Annotation - SBO Terms | Gene General SBO Presence | 0.0% | +| Annotation - SBO Terms | Gene SBO:0000243 Presence | 0.0% | +| Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 0.0% | ## Full suite diff --git a/data/testResults/model_qc_summary.md b/data/testResults/model_qc_summary.md index 9f3c7613..393dffaf 100644 --- a/data/testResults/model_qc_summary.md +++ b/data/testResults/model_qc_summary.md @@ -45,7 +45,51 @@ _Duplicate keys (model unloadable) and no growth block the merge; every other ro ### [MEMOTE](https://github.com/SysBioChalmers/Human-GEM/blob/fix/qc-comment-running-state/data/testResults/README.md#memote) -_running_ · :hourglass_flowing_sand: +**Total score: 63.2%** (core subset)   +43.0 :white_check_mark: + +| Section | Score | Δ vs base | +| --- | ---: | ---: | +| consistency | 42.4% | 0 | +| annotation_met | 73.0% | +48.0 :white_check_mark: | +| annotation_rxn | 72.7% | +47.7 :white_check_mark: | +| annotation_gene | 46.7% | +46.7 :white_check_mark: | +| annotation_sbo | 81.7% | +81.7 :white_check_mark: | + +
Per-test scores + +| Section | Test | Score | +| --- | --- | ---: | +| Consistency | Stoichiometric Consistency | 100.0% | +| Consistency | Mass Balance | 0.8% | +| Consistency | Charge Balance | 2.1% | +| Consistency | Metabolite Connectivity | 0.0% | +| Consistency | Unbounded Flux In Default Medium | 100.0% | +| Annotation - Metabolites | Presence of Metabolite Annotation | 0.0% | +| Annotation - Metabolites | Metabolite Annotations Per Database | 62.3% | +| Annotation - Metabolites | Metabolite Annotation Conformity Per Database | 45.8% | +| Annotation - Metabolites | Uniform Metabolite Identifier Namespace | 0.0% | +| Annotation - Reactions | Presence of Reaction Annotation | 0.0% | +| Annotation - Reactions | Reaction Annotations Per Database | 75.9% | +| Annotation - Reactions | Reaction Annotation Conformity Per Database | 33.3% | +| Annotation - Reactions | Uniform Reaction Identifier Namespace | 0.0% | +| Annotation - Genes | Presence of Gene Annotation | 0.0% | +| Annotation - Genes | Gene Annotations Per Database | 80.0% | +| Annotation - Genes | Gene Annotation Conformity Per Database | 80.0% | +| Annotation - SBO Terms | Metabolite General SBO Presence | 0.0% | +| Annotation - SBO Terms | Metabolite SBO:0000247 Presence | 0.1% | +| Annotation - SBO Terms | Reaction General SBO Presence | 0.0% | +| Annotation - SBO Terms | Metabolic Reaction SBO:0000176 Presence | 0.0% | +| Annotation - SBO Terms | Transport Reaction SBO:0000185 Presence | 0.7% | +| Annotation - SBO Terms | Exchange Reaction SBO:0000627 Presence | 0.0% | +| Annotation - SBO Terms | Demand Reaction SBO:0000628 Presence | 100.0% | +| Annotation - SBO Terms | Sink Reactions SBO:0000632 Presence | 100.0% | +| Annotation - SBO Terms | Gene General SBO Presence | 0.0% | +| Annotation - SBO Terms | Gene SBO:0000243 Presence | 0.0% | +| Annotation - SBO Terms | Biomass Reactions SBO:0000629 Presence | 0.0% | + +
+ +_Full suite not run for this commit; comment_ `/run memote` _to add it._ _The score above is the fast core subset. Comment_ `/run memote` _to run the full suite on this pull request; the score updates here when it finishes._ From 6b573760e97fd5f13e1c1b992ab379356ee8a745 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Thu, 16 Jul 2026 00:59:47 +0200 Subject: [PATCH 10/11] fix: keep the Human-GEM.mat variable named humanGEM export_for_git writes the .mat with cobra's default variable name (the model id, HumanGEM). Write the plain YAML/MATLAB exports explicitly instead - YAML via raven-toolbox, MATLAB via cobra with varname=humanGEM - and keep export_for_git for the annotated xml/xlsx/txt exports. --- code/io/increaseHumanGEMVersion.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/code/io/increaseHumanGEMVersion.py b/code/io/increaseHumanGEMVersion.py index 568275b2..a62d84bd 100644 --- a/code/io/increaseHumanGEMVersion.py +++ b/code/io/increaseHumanGEMVersion.py @@ -36,7 +36,7 @@ sys.path.insert(0, str(REPO_ROOT / "code")) from annotateGEM import annotate_gem # noqa: E402 -from raven_toolbox.io import export_for_git, read_yaml_model # noqa: E402 +from raven_toolbox.io import export_for_git, read_yaml_model, write_yaml_model # noqa: E402 # model attribute <-> TSV file <-> id column, for the consistency check. _ID_TABLES = ( @@ -133,12 +133,14 @@ def increase_human_gem_version(bump_type: str, test: bool = False) -> str | None _check_tsv_consistency(model) - # Export via raven-toolbox's Standard-GEM writer. The plain formats (yml/mat) - # keep their cross-references in the TSV tables; the annotated formats (xml/xlsx/ - # txt) carry the merged TSV cross-references and SBO terms (see annotateGEM.py). - # export_for_git also (re)writes model/dependencies.txt. - export_for_git(model, MODEL_DIR, prefix="Human-GEM", - formats=("yml", "mat"), sub_dirs=False) + # Plain exports keep their cross-references in the TSV tables. YAML via + # raven-toolbox; MATLAB via cobra with the "humanGEM" variable name (which + # export_for_git would not set - it uses cobra's default, the model id). + write_yaml_model(model, MODEL_DIR / "Human-GEM.yml") + cobra.io.save_matlab_model(model, str(MODEL_DIR / "Human-GEM.mat"), varname="humanGEM") + + # Annotated exports (xml/xlsx/txt) carry the merged TSV cross-references and SBO + # terms (see annotateGEM.py); export_for_git also (re)writes dependencies.txt. export_for_git(annotate_gem(model.copy(), MODEL_DIR), MODEL_DIR, prefix="Human-GEM", formats=("xml", "xlsx", "txt"), sub_dirs=False) From 04aaf5c78f792388dbaed0bd73eebc86a1003473 Mon Sep 17 00:00:00 2001 From: Eduard Kerkhoven Date: Thu, 16 Jul 2026 09:29:43 +0200 Subject: [PATCH 11/11] refactor: pin the .mat variable via export_for_git's varname Revert the increaseHumanGEMVersion workaround (explicit write_yaml_model + save_matlab_model) now that raven-toolbox's export_for_git takes a varname argument. The plain yml/mat export is a single export_for_git call again, with varname='humanGEM' pinning the MATLAB struct name. --- code/io/increaseHumanGEMVersion.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/code/io/increaseHumanGEMVersion.py b/code/io/increaseHumanGEMVersion.py index a62d84bd..1781bda7 100644 --- a/code/io/increaseHumanGEMVersion.py +++ b/code/io/increaseHumanGEMVersion.py @@ -36,7 +36,7 @@ sys.path.insert(0, str(REPO_ROOT / "code")) from annotateGEM import annotate_gem # noqa: E402 -from raven_toolbox.io import export_for_git, read_yaml_model, write_yaml_model # noqa: E402 +from raven_toolbox.io import export_for_git, read_yaml_model # noqa: E402 # model attribute <-> TSV file <-> id column, for the consistency check. _ID_TABLES = ( @@ -133,14 +133,13 @@ def increase_human_gem_version(bump_type: str, test: bool = False) -> str | None _check_tsv_consistency(model) - # Plain exports keep their cross-references in the TSV tables. YAML via - # raven-toolbox; MATLAB via cobra with the "humanGEM" variable name (which - # export_for_git would not set - it uses cobra's default, the model id). - write_yaml_model(model, MODEL_DIR / "Human-GEM.yml") - cobra.io.save_matlab_model(model, str(MODEL_DIR / "Human-GEM.mat"), varname="humanGEM") - - # Annotated exports (xml/xlsx/txt) carry the merged TSV cross-references and SBO - # terms (see annotateGEM.py); export_for_git also (re)writes dependencies.txt. + # Export via raven-toolbox's Standard-GEM writer. The plain formats (yml/mat) + # keep their cross-references in the TSV tables; the annotated formats (xml/xlsx/ + # txt) carry the merged TSV cross-references and SBO terms (see annotateGEM.py). + # export_for_git also (re)writes model/dependencies.txt. varname pins the .mat + # struct name to "humanGEM". + export_for_git(model, MODEL_DIR, prefix="Human-GEM", + formats=("yml", "mat"), sub_dirs=False, varname="humanGEM") export_for_git(annotate_gem(model.copy(), MODEL_DIR), MODEL_DIR, prefix="Human-GEM", formats=("xml", "xlsx", "txt"), sub_dirs=False)