Mo edits - #15
Merged
Merged
Conversation
added 10 commits
June 26, 2026 16:18
Allows manually running CI on a branch without needing to open a PR first.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ZipStrain` Nextflow only auto-loads a file named `nextflow.config`, so `conf.config` was never applied when running the pipeline from GitHub. Add a root nextflow.config that: - enables Docker by default (with --platform linux/amd64 for Apple Silicon) - sets the default container to parsaghadermazi/zipstrain:0.11.1 - declares zipstrain.nf as the manifest mainScript (no -main-script needed) - includeConfig 'conf.config' for per-process resources and HPC profiles Also disable Docker in the singularity profiles (alpine/gutbot/blanca/fiji) so selecting one does not conflict with the new Docker default. This lets a laptop run work with just: nextflow run OlmLab/ZipStrain --mode ... --input_table ... --output_dir ... -resume Co-Authored-By: Claude Opus 4.8 <[email protected]>
Every top-level run now writes zipstrain_run.log (human-readable, append-only timeline) and zipstrain_status.json (machine-readable status) into its output directory, so users can tell whether a run is still running, completed, or crashed — and, on a crash, which step was running plus the full traceback. - New zipstrain/run_logger.py: RunLogger context manager (running -> completed, or crashed/aborted on exception; atomic status writes; never raises). - Wire map, profile, and compare (incl. the matrix route) to log their steps. - Tests in test_run_logger.py. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…g and map resume - Rewrite installation.md into Python CLI vs Nextflow sections, each with its own requirements, confirmed-working versions, and troubleshooting. - Restructure the CLI reference in usermanual.md around map/profile/compare/ utilities with collapsible full --help blocks; document the run-logging files and map's resume behavior; add CLI-vs-Nextflow pros/cons. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…onfig The new root nextflow.config enables Docker by default and includes conf.config (which sizes compare processes at 8 cpus). Those defaults now auto-load for `nextflow run zipstrain.nf`, breaking the deliberately container-free compare_genomes end-to-end test on 4-core CI runners. Pass a small `-c` override that disables Docker and caps the two compare processes to 1 cpu / 512 MB, mirroring the existing Docker-variant test, so the test runs locally with the `zipstrain` CLI on PATH as intended. Co-Authored-By: Claude Opus 4.8 <[email protected]>
- Add description, second author, keywords, classifiers, and project URLs; pin the matrix extra (torch >=2.6,<3.0.0, h5py >=3.8). - Refresh the PyPI long-description with the three-command intro, the conda install path, and an Apple Silicon (native osx-arm64) note. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…usage - Remove dead pages (cli.md, UNUSED.md); promote tutorial_v2.md to Tutorial.md as the single canonical tutorial; fix all cross-links and the nav. - Short one-page Quick Start + README with map/profile/compare gifs. - Modernize all Nextflow examples to `nextflow run OlmLab/ZipStrain` (Docker by default) and document pre-downloading the Sylph database via --sylph_db. - Installation: Apple Silicon (native osx-arm64) guidance and channel hygiene. - Remove the stray overview-figure TODO. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…I, StB fix) Profiling defaults now lean toward inStrain to cut mis-mapping-driven SNV calls: - --min-read-ani defaults to 0.95 (reads without an NM tag are kept, not errored) - --read-inclusion defaults to 'paired', made single-end-safe (keeps single-end reads and concordant pairs; drops only half-mapped orphans) Robustness: - read_stb() strips stray whitespace from StB files (padded StBs no longer send scaffolds silently to genome 'NA'); wired through profiling + compare loads New inStrain-parity outputs: - genome_stats: conANI_reference (consensus ref-ANI, alongside ref_ani = popANI-reference), coverage_median, coverage_std - compare: conANI via a new 'conani' calculation -> share_consensus_pos, consensus_SNPs, genome_con_ani, in both the Polars and DuckDB engines. Matrix mode rejects conani (its presence-only store can't recover consensus); the matrix hot path is untouched. Docs: document the new defaults, the 0-based(inStrain)/1-based(ZipStrain) SNV coordinate convention, conANI_reference, and the compare consensus columns. Co-Authored-By: Claude Opus 4.8 <[email protected]>
The per-sample SNVs table now classifies every divergent site in inStrain's style (columns: position_coverage, allele_count, ref/con/var_base + freqs, cryptic, class) instead of emitting only ref-divergent rows. Since profile counts are error-adjusted, a base with count>0 is a passing allele; with alleles/con/ref, sites are labelled SNS / SNV / con_SNV / pop_SNV (rules reverse-engineered from real inStrain output). genome_stats gains SNS_count and SNV_count (reference route), which sum exactly to the SNV table's SNS and SNV+con_SNV+pop_SNV totals per genome. Docs updated with the full SNV schema, the class definitions, and the counts. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…ods) percent_compared = total_positions / genome_length; coverage_overlap = total_positions / positions-covered-in-either. Both are computed as a post-processing join on the final all_comparisons.parquet, so they work for the standard and matrix methods without touching either compare compute path: - percent_compared uses the genome_lengths.parquet asset (auto-discovered from profiling_assets) - coverage_overlap uses each sample's genome_stats (Nx_cov_sites), siblings of the profiles; exact when --min-cov matches the profiling coverage cutoff Both no-op gracefully when their inputs aren't available. Documented in the compare output table. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…fault profile.py profiled BAM chunks with asyncio subprocesses. asyncio's default ThreadedChildWatcher spawns one waitpid thread per child, and profiling launches many short-lived samtools mpileup processes per chunk. Under concurrent invocation these watcher threads accumulate (100s of idle threads) and asyncio.run()'s shutdown (_cancel_all_tasks) deadlocks, hanging the profiler. Replace the asyncio subprocess + event-loop orchestration with synchronous subprocess calls driven by a ThreadPoolExecutor. subprocess reaps its own children via waitpid, so no child watcher is involved and no threads accumulate. Also build the FASTA .fai once up front to avoid a concurrent index-creation race between chunk mpileups. Profiling output is byte-identical to before; verified 36/36 concurrent runs complete with no deadlock (previously the profiler required -n 1 to avoid hanging). Separately, tighten the default read-inclusion 'paired' mode to match inStrain's default 'paired_only': a paired read is kept only if its mate maps to the same scaffold (SAM RNEXT == "=" / RNAME), dropping half-mapped orphans and cross-scaffold pairs that otherwise inflate coverage relative to inStrain. Genuinely single-end reads are still kept (single-end-safe). Co-Authored-By: Claude Opus 4.8 <[email protected]>
…SNP VCF) main (v0.11.2/0.11.3) independently fixed the profiling stalling bug and added a reference-SNP VCF export. Resolve the merge keeping mo-edits' code base (self-consistent, with its own stalling fix + inStrain-parity SNV work + read-inclusion same-scaffold default) and taking main's docs/version/dockerfile/ changelog/visualize updates. Port main's reference-SNP VCF capability onto the mo-edits base: - utils: add REF_BASE_FROM_BITMASK + decode_reference_base_bitmask (bit-compatible with mo-edits' existing encode). - profile: add VCF_SNP_BASE_ORDER and write_reference_snps / _write_reference_snps_ vcf_from_parquet / _reference_snp_alt_bases. - cli: upgrade `utilities get-snp-reference` with `--fmt parquet|vcf`. NOTE: the VCF is rendered from get_reference_snps, which mo-edits rewrote into a richer inStrain-parity table, so the VCF site set is a superset of main's original (includes SNV sites where consensus==reference). The exact SNP-definition semantics are left to a follow-up decision. Full test suite passes (311). Co-Authored-By: Claude Opus 4.8 <[email protected]>
Adopt the robustness of main's stalling-fix approach on top of mo-edits' synchronous-subprocess profiler: launch the samtools mpileup child with start_new_session=True (its own process group), wait with a PROFILE_SUBPROCESS_TIMEOUT_SECONDS ceiling, and SIGKILL the whole process group via _kill_process_group on timeout or any abnormal exit so a hung samtools can never leak orphaned processes. Belt-and-suspenders on top of the async->sync fix that removed the child-watcher deadlock. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Lock in the chosen semantics for `get-snp-reference --fmt vcf`: it emits a record for any covered site carrying a non-reference allele, including SNV sites where the reference base is still the consensus (a minor variant is present). This is the superset of main's original consensus!=reference definition, matching mo-edits' richer inStrain-parity get_reference_snps. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Prepare the first stable release.
- Bump version to 1.0.0 (pyproject.toml, CITATION.cff) and add a
comprehensive 1.0.0 changelog entry.
- Fix `zipstrain compare --calculate`: the batched standard path rejected
any spec other than {all, ani}, even though the per-pair
single_compare_genome subprocess it drives understands '+'/','-joined
combos (e.g. ani+ibs+identical_genes) and --help advertises them.
Validate via compare.parse_genome_calculations instead, and add a
regression test.
- Docs: delete the orphaned, superseded cli.md and NextflowPipeline.md
(usermanual.md is now the canonical CLI + Nextflow reference) and
repoint their links; update Tutorial.md to the current unified CLI
(`zipstrain compare` / `--compare-genes` instead of the removed
`compare genomes`/`compare genes` subcommands) and refresh comparison
output-column lists (conANI, percent_compared, coverage_overlap);
fix broken Tutorial anchors in expected_output.md.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
The published Tutorial.md had regressed to an old, pre-CLI-migration version; the current numbered walkthrough (Tutorial #1 Python CLI / Tutorial #2 Nextflow+SRA / Tutorial #3 Sylph + matrix compare on the ZymoBIOMICS standard) had been dropped when docs/tutorial_v2.md was deleted in 95b5323. Resurrect that content (from 95b5323^:docs/tutorial_v2.md) as the canonical docs/Tutorial.md, replacing the stale version. It already uses the current unified CLI (`zipstrain map | profile | compare`, `--method matrix`, `--compare-genes`) so it supersedes the incremental fixes made to the old file in the previous commit. Also restore the expected_output.md cross-links to `Tutorial.md#interpreting-the-benchmarks`, whose target section now exists again. `mkdocs build --strict` passes. Co-Authored-By: Claude Opus 4.8 <[email protected]>
MrOlm
marked this pull request as ready for review
July 13, 2026 21:32
Contributor
|
Incorporating edits from Matt |
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
2 participants
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.
Don't actually accept this- I just want to see if the CI tests are working