Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical integration and failure-handling issues, plus output, resource, and test contract problems, remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds read- and insert-level Tasmanian positional counts with a pair_mode column.
Changes:
- Runs Tasmanian in read and insert modes.
- Combines results into a TSV with mode labels.
- Updates dependencies, inputs, and process configuration.
File summaries
| File | Review findings |
|---|---|
modules/tasmanian.nf |
Critical (3): Required inputs are not wired correctly; the FAI is passed as a mask bedgraph. Output emit name mismatches the caller. Failures may be masked by disabled error propagation. Moderate (3): process_low lacks a configured selector. Moderate (2): Publication and filename changes break the existing output contract. Nit (1): Tests do not validate the new pair_mode values. |
Review details
Suppressed comments (1)
modules/tasmanian.nf:45
- The new read/insert merge is not covered by the existing nf-tests: they still look for
*.tasmanian.csvand only assert file existence, rather than checking the new header andread/insertvalues. Update both test cases and snapshots to the.tsvpath and assert representative rows contain the newpair_modevalues, otherwise this behavior can regress unnoticed.
head -n1 ${library}.tasmanian.read.tsv | awk '{print $0"\tpair_mode"}' > ${library}.tasmanian.tsv
tail -n +2 ${library}.tasmanian.read.tsv | awk '{print $0"\tread"}' >> ${library}.tasmanian.tsv
tail -n +2 ${library}.tasmanian.insert.tsv | awk '{print $0"\tinsert"}' >> ${library}.tasmanian.tsv
- Files reviewed: 1/1 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+10
to
+11
| each path(reference_fasta) | ||
| each path (masked_bedgraph) |
|
|
||
| output: | ||
| tuple val(library), path("${library}.tasmanian.csv"), emit: for_agg | ||
| tuple val(library), path("${library}.tasmanian.tsv"), emit: tasmanian_for_aggregate |
Comment on lines
20
to
21
| set +e | ||
| set +o pipefail |
| maxRetries 1 | ||
| memory { task.attempt > 1 ? '16 GB' : '8 GB' } | ||
| tag "${library}" | ||
| label 'process_low' |
|
|
||
| output: | ||
| tuple val(library), path("${library}.tasmanian.csv"), emit: for_agg | ||
| tuple val(library), path("${library}.tasmanian.tsv"), emit: tasmanian_for_aggregate |
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
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.
Adds pair_mode column to tasmanian output, distinguishing read vs insert level positional counts.