PR: Wire report notebooks into pipeline#93
Open
KevinMLanderos wants to merge 4 commits into
Open
Conversation
Extends RENDER_NOTEBOOK/REPORT wiring to template_discovery_brief.qmd,
template_details_part1.qmd, and template_details_part2.qmd, and fixes
bugs surfaced by actually executing all 7 underlying notebooks
(template_sample, template_overlap, template_sharing, template_giana,
template_gliph, template_pheno_sc, template_pheno_bulk) against real
pipeline-generated data, including a full end-to-end Nextflow run,
rather than assuming correctness from static review.
Pipeline wiring:
- Add named emits for sample/patient/compare subworkflow outputs needed
by the new reports (v_family, j_family, tcrdist/olga/vdjdb files,
convergence files, tcrpheno files, shared_cdr3, giana/gliph2 outputs).
- Stage {{< include >}} sibling .qmd files alongside their parent
notebook in RENDER_NOTEBOOK, since Nextflow only auto-stages the
named notebook file.
- Give GLIPH2_TURBOGLIPH outputs patient-prefixed filenames
(_all_motifs.txt, etc.) to avoid Nextflow's fatal
input-file-name collision when staging multiple patients' outputs
together, matching the existing GIANA_CALC convention. Notebooks read
these prefixed names directly instead of requiring a rename step.
- Wire CONVERT.out.sample_map_converted into the discovery brief's
staged files so the VDJdb section reads AIRR-converted files instead
of falling back to raw adaptive TSVs.
- Make template_discovery_brief.qmd's phenotype section conditional:
a single generic {{< include ./template_pheno.qmd >}} is spliced in,
and the pipeline stages whichever real notebook applies under that
name - template_pheno_sc.qmd for cellranger input with sobject_gex
supplied, template_pheno_bulk.qmd otherwise - since Quarto's include
shortcode has no native runtime conditional.
- Register template_pheno_sc/template_pheno_bulk as pipeline params.
Notebook fixes (found via end-to-end render testing):
- Fix systemic legacy-vs-AIRR column naming bug (CDR3b/counts/TRBV/TRBJ
vs junction_aa/duplicate_count/v_call/j_call) across
template_discovery_brief, template_sample, template_overlap, and
template_sharing, wherever the data originates from AIRR/concat
outputs. Left legacy names intact where they genuinely belong
(GIANA/GLIPH2 module outputs, tcrdist3/sample_stats schemas).
- Fix convergence file merge silently producing zero rows by handling
the filename suffix when parsing sample IDs.
- Fix 3 VDJdb plotting functions and template_pheno_bulk.qmd's
phenotype-composition functions silently using hardcoded column
names (subject_id/alias/timepoint_order) instead of the notebook's
configured subject_col/alias_col/timepoint_order_col.
- Fix VDJdb converted-file lookup to match by sample name (matching how
CONVERT_ADAPTIVE actually names its outputs) instead of reconstructing
a filename from the raw input basename.
- Fix template_pheno_bulk.qmd's weighted-phenotype merge, which joined
TCRPHENO scores against clone counts on a CDR3/junction_aa column
that TCRPHENO's real R-script output never contains - confirmed by
running the actual tcrpheno R package - and rewrite it to join on
sequence_id, which both files share.
- Point template_pheno_sc.qmd's phenotype clonotype source at
annotate/concatenated_cdr3_sorted.tsv (matching the rest of the
pipeline) instead of a compare_phenotype/ path no module produces,
and remove dead samplesheet_phenotype.csv loading code.
- Remove genuinely dead imports (logomaker and others) across all
notebooks, verified individually via pyflakes plus manual chain
tracing to avoid removing imports still needed by earlier-executing
cells in the Quarto include splice.
Testing:
- Update render_notebook.nf.test for the new [dest, src] staged_layout
tuple format, with cases for nested paths and renamed basenames.
2=Post), required by timepoint_order_col in the report notebooks. Update
the matching header assertion in samplesheet_check.nf.test.
- Make GIANA/GLIPH2 inclusion in template_details_part2.qmd conditional on
the patient workflow_level: splice in a single generic
template_patient_clustering.qmd, which workflows/tcrtoolkit.nf resolves to
either template_patient_clustering_on.qmd (includes both notebooks) or a
placeholder off.qmd, mirroring the existing template_pheno.qmd mechanism.
Avoids a hard crash in template_giana.qmd when patient clustering never
ran, and a wall of "no data" warnings otherwise.
- Fix subworkflows/local/patient.nf's gliph2_* emits: plain .collect() on
tuple(val(patient), path) channels flattens the tuples by default,
corrupting downstream [patient, file] pair indexing (surfaced as a cryptic
"No such variable: name" error). Fixed with .collect(flat: false).
- Handle GLIPH2/GIANA finding no significant results, which isn't always a
clean "no results" case but can also leave output files missing or
malformed:
- gliph2.nf: wrap turboGliph::gliph2() in tryCatch (it can throw an
uncaught internal dplyr type-mismatch error) and backfill empty
placeholders for any of its output files left missing.
- template_giana.qmd: catch EmptyDataError when GIANA writes only
metadata comments and no data for a patient, at all 3 read sites, and
guard the final concat against an all-empty patient list.
- template_gliph.qmd: skip genuinely 0-byte output files (not just
missing ones), which also crash pd.read_csv.
- Set embed-resources: true in template_details_part1/part2.qmd, matching
the other reports - RENDER_NOTEBOOK only publishes the bare .html, so the
separate CSS/JS assets Quarto writes under embed-resources: false were
never shipped, leaving these two reports unstyled.
Unit Test Results13 tests 13 ✅ 4m 0s ⏱️ Results for commit 78bb82f. ♻️ This comment has been updated with latest results. |
These were created in 3b8621e but silently blocked from git add by the stale `notebooks/*` .gitignore rule, so they only ever existed on disk locally. CI checks out from git, so the RENDER_NOTEBOOK symlink for template_patient_clustering.qmd pointed at a nonexistent file, and Quarto's {{< include >}} failed with "could not find file". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires 3 more report notebooks (discovery brief, patient details part 1/2) into the pipeline, then fixes a long list of bugs that only showed up once the notebooks were actually executed end-to-end against real pipeline output (rather than reviewed statically) via a full Nextflow run.
Pipeline wiring
Added named emits for sample/patient/compare outputs needed by the new reports (v/j family, tcrdist/olga/vdjdb, convergence, tcrpheno, shared_cdr3, giana/gliph2).
Fixed Nextflow staging so sibling {{< include >}}'d .qmd files travel with their parent notebook.
Made phenotype and patient-clustering sections conditional via generic include shims (template_pheno.qmd, template_patient_clustering.qmd) that resolve to the right notebook (or an "off" placeholder), since Quarto includes have no native runtime conditional.
Prefixed GLIPH2_TURBOGLIPH output filenames per-patient to avoid Nextflow file-collision errors when staging multiple patients together.
Bugs found via real execution
Fixed legacy-vs-AIRR column naming mismatches (CDR3b/TRBV/etc. vs junction_aa/v_call/etc.) across several notebooks.
Fixed convergence-file merge silently returning zero rows.
Fixed several plotting/composition functions ignoring configured column names in favor of hardcoded ones.
Fixed VDJdb file lookups and the phenotype-scoring join key, both of which were built on assumptions that didn't match the real module outputs.
Robustness / error handling
Wrapped GLIPH2 in tryCatch and backfilled placeholder outputs when it finds no significant results or errors internally.
Added handling for empty/malformed/0-byte GIANA and GLIPH2 outputs instead of crashing.
Fixed a .collect() bug that was corrupting patient/file pairing in a subtle way.
Enabled embed-resources on the two new reports so they ship styled.
Testing
Updated render_notebook.nf.test for the new staged-layout tuple format.
Updated the minimal-example samplesheet and its test assertions for the new required timepoint_order column.