-
Notifications
You must be signed in to change notification settings - Fork 24
mosdepth cov stat #454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
mosdepth cov stat #454
Conversation
Added evenness score to CoverageStats
… if statement to avoid evenness score if mean is 0
…t float arithmetic
# Conflicts: # .dockstore.yml
…CoverageStats workflow, shifted to using feature branch for docker of mosdepth"
… vms, this should automatically be scaled by mem"
…nterval to be processed my mosdepth per-base"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new WDL workflow to compute coverage statistics using mosdepth and registers it in Dockstore.
- Adds
MosdepthCoverageStats
workflow with options for summary and per-interval stats, including filtering and binning. - Implements three core tasks:
MosDepthOverBed
,MosDepthPerInterval
, andBinBed
, plus a failure handler. - Updates
.dockstore.yml
to include the newCoverageStats
workflow.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
wdl/pipelines/TechAgnostic/Utility/CoverageStats.wdl | New workflow and tasks to generate coverage stats via mosdepth. |
.dockstore.yml | Registers the CoverageStats workflow for Dockstore publishing. |
Comments suppressed due to low confidence (2)
wdl/pipelines/TechAgnostic/Utility/CoverageStats.wdl:6
- [nitpick] The workflow description could be expanded to briefly outline the output artifacts (e.g., CSV and JSON summary files) and key metrics calculated for better user guidance.
description: "Calculate coverage statistics using mosdepth."
wdl/pipelines/TechAgnostic/Utility/CoverageStats.wdl:3
- There are no accompanying tests or validation workflows for the new MosdepthCoverageStats workflow. Consider adding WDL unit or integration tests to verify key branches (e.g., with and without bed files and stats_per_interval).
workflow MosdepthCoverageStats {
if (defined(bed_file)) { | ||
if (length(read_lines(select_first([bed_file]))) == 0) { | ||
call FailWorkflow as EmptyBedFile { | ||
input: | ||
message = "stats_per_interval is set to true, but the provided bed file is empty." | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty bed-file check is applied whenever bed_file is defined, even if stats_per_interval is false. Consider moving this validation inside the stats_per_interval block or adding a condition to only fail when stats_per_interval is true.
if (defined(bed_file)) { | |
if (length(read_lines(select_first([bed_file]))) == 0) { | |
call FailWorkflow as EmptyBedFile { | |
input: | |
message = "stats_per_interval is set to true, but the provided bed file is empty." | |
} | |
} | |
} | |
# Empty bed file check moved into stats_per_interval block. |
Copilot uses AI. Check for mistakes.
} | ||
} | ||
|
||
task MosDepthPerInterval { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The MosDepthOverBed and MosDepthPerInterval tasks share many duplicated inputs and parameters. Consider factoring common logic into a separate subtask or parameterized call to reduce duplication and simplify maintenance.
Copilot uses AI. Check for mistakes.
Adding workflow that generates a set of coverage stats for a BAM file using Mosdepth.
Output results to a CSV file and JSON value
Output stat includes: