feat!: Pin wrapper versions in meta-wrappers; add alignoth_report meta-wrapper #4678
Conversation
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
- Update rules to use compressed VCF files and overview tables - Refactor helper functions for variant counting and position lookup - Switch datavzrd config to use template and new dataset structure - Remove unused config.yaml and relocate test data files
Contributor
📝 WalkthroughWalkthroughAdds a new meta-wrapper alignoth_report (metadata, Snakemake module, datavzrd template, tests, used_wrappers), extends test harness to support meta-wrappers, pins many meta-wrapper wrapper paths to explicit versions, and updates meta-wrapper tests to load local files and paths. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as test_wrappers.py
participant Tmp as Temp Dir
participant SMK as Snakemake
Note right of Test `#dfeffb`: Resolve wrapper path and classify meta vs non-meta
Test->>Test: compute is_meta_wrapper (wrapper.startsWith("meta/"))
alt meta wrapper
Test->>Tmp: copy meta wrapper files into temp dir
Test->>SMK: run snakemake (no wrapper-prefix) with common flags
else non-meta wrapper
Test->>SMK: run snakemake with wrapper-prefix -> git+file://{tmp}/ and common flags
end
SMK->>SMK: load local meta_wrapper.smk and resources
SMK->>SMK: checkpoint vembrane_table -> produce tables/{sample}.tsv
SMK->>SMK: run alignoth per-sample/index -> produce alignoth/{sample}/{index}/ (plots & .vl.json)
SMK->>SMK: run datavzrd -> render datavzrd-report/ (HTML)
SMK-->>Test: return outputs/logs
sequenceDiagram
autonumber
participant Workflow as Snakemake
participant Vembrane as Vembrane Wrapper
participant Alignoth as Alignoth Wrapper
participant Datavzrd as Datavzrd Wrapper
Workflow->>Vembrane: checkpoint vembrane_table (BCF -> per-sample TSV)
Vembrane-->>Workflow: tables/{sample}.tsv
Workflow->>Alignoth: alignoth rule (BAM + VCF/CSI + ref -> per-index plots & .vl.json)
Alignoth-->>Workflow: alignoth/{sample}/{index}/ (plots & .vl.json)
Workflow->>Datavzrd: datavzrd rule (overview + plot_tables -> render HTML)
Datavzrd-->>Workflow: datavzrd-report/ (HTML)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
meta/bio/alignoth_report/meta_wrapper.smk (1)
59-61: Tiny guardrail incount_variants.Protect against empty files returning negative counts.
Apply:
def count_variants(wildcards): - return sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 + n = sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 + return max(n, 0)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
meta/bio/alignoth_report/meta.yaml(1 hunks)meta/bio/alignoth_report/meta_wrapper.smk(1 hunks)meta/bio/alignoth_report/resources/template.datavzrd.yaml(1 hunks)meta/bio/alignoth_report/test/Snakefile(1 hunks)meta/bio/alignoth_report/test/resources/genome.fa(1 hunks)meta/bio/alignoth_report/test/resources/genome.fa.fai(1 hunks)meta/bio/alignoth_report/used_wrappers.yaml(1 hunks)test_wrappers.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
Files:
test_wrappers.py
🪛 GitHub Actions: Code quality
meta/bio/alignoth_report/test/Snakefile
[error] 8-8: WorkflowError in file "/home/runner/work/snakemake-wrappers/snakemake-wrappers/meta/bio/alignoth_report/test/Snakefile", line 8: Invalid meta wrapper master/meta/bio/alignoth_report: Could not find meta_wrapper.smk or test/Snakefile (old style).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (5)
meta/bio/alignoth_report/test/resources/genome.fa.fai (1)
1-1: LGTM for test fixture.Minimal FAI content is fine for this test resource.
meta/bio/alignoth_report/used_wrappers.yaml (1)
1-4: Wrapper list OK.Matches the meta-wrapper’s rules.
meta/bio/alignoth_report/test/resources/genome.fa (1)
1-2: LGTM for test fixture.Suitable minimal FASTA for tests.
test_wrappers.py (1)
259-269: Test addition looks correct.Target aligns with
datavzrdrule output. Failures are due to the test Snakefile module path (see separate comment).meta/bio/alignoth_report/resources/template.datavzrd.yaml (1)
1-58: Template looks consistent with inputs and views.YTE vars and per-index views align with meta-wrapper functions.
Ensure
params.max_indexequals the number of variant rows (no off-by-one).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
meta/bio/alignoth_report/meta_wrapper.smk (3)
1-1: Avoid global pandas import in meta-wrapper.Meta-wrappers run in the host Python environment where pandas may not be available. This issue was previously flagged but remains unresolved.
53-56: Bug: unformatted{sample}in returned paths.This critical issue was previously flagged but remains unresolved. The literal
{sample}prevents Snakemake from correctly matchingalignothoutputs.
63-68: Remove pandas dependency and add bounds check inget_variant_position.This issue was previously flagged but remains unresolved. The pandas dependency is problematic for meta-wrappers, and bounds checking is missing.
🧹 Nitpick comments (2)
meta/bio/alignoth_report/meta_wrapper.smk (2)
44-44: Remove commented code.Line 44 contains commented-out code that should either be removed or uncommented if still needed.
59-60: Consider adding bounds checking.The function works correctly but could be more robust by handling edge cases like empty files or ensuring at least one data row exists.
Apply this diff to add a bounds check:
def count_variants(wildcards): - return sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 + count = sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 + if count < 0: + raise ValueError(f"No variants found for sample {wildcards.sample}") + return count
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
meta/bio/alignoth_report/meta_wrapper.smk(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: docs
- GitHub Check: testing
- GitHub Check: Summary
🔇 Additional comments (3)
meta/bio/alignoth_report/meta_wrapper.smk (3)
3-14: LGTM: Checkpoint structure is correct.The
vembrane_tablecheckpoint is properly defined with appropriate input/output paths and parameters for extracting variant data from BCF to TSV.
17-31: LGTM: Rule structure is correct.The
alignothrule is properly defined with appropriate inputs, outputs, and the dynamic parameter building viaget_variant_position. Note that this rule depends on fixing the pandas issue inget_variant_position(flagged separately).
34-43: LGTM: Rule structure is correct.The
datavzrdrule is properly defined with dynamic input dependencies and appropriate use ofworkflow.source_path()for accessing the template configuration.Also applies to: 45-50
Removed plot configuration for REF and ALT labels, added spell URL.
…pers into alignoth-report
johanneskoester
requested changes
Oct 30, 2025
Updated the extra parameter in the Snakemake wrapper to use the variant index instead of calculating the position from the overview input. Removed the get_variant_position function as it is no longer needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
meta/bio/alignoth_report/meta_wrapper.smk (1)
53-56: CRITICAL: Fix unformatted{sample}placeholder in returned paths.This is the same bug flagged in the previous review. The double braces
{{sample}}produce a literal"{sample}"string instead of the actual sample name, preventing Snakemake from matching the alignoth outputs.Apply this diff:
def get_alignoth_tables(wildcards, results_dir): count = count_variants(wildcards) - l = [f"{results_dir}/alignoth/{{sample}}/{i}/" for i in range(count)] + l = [f"{results_dir}/alignoth/{wildcards.sample}/{i}/" for i in range(count)] return l
🧹 Nitpick comments (2)
meta/bio/alignoth_report/meta_wrapper.smk (2)
1-1: Remove unused pandas import.The
pandasimport is not used anywhere in this file and should be removed.Apply this diff:
-import pandas as pd - checkpoint vembrane_table:
44-44: Remove commented-out code.Commented code should be removed to keep the file clean.
Apply this diff:
), - # config = "resources/datavzrd/{sample}.rendered_config.yaml" params:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
meta/bio/alignoth_report/meta_wrapper.smk(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-12-06T14:25:43.922Z
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3498
File: bio/ngscheckmate/ncm/wrapper.py:68-68
Timestamp: 2024-12-06T14:25:43.922Z
Learning: In the `bio/ngscheckmate/ncm/wrapper.py` file for the NGSCheckMate wrapper, do not suggest adding file existence checks for the list file input, as the files are already required by the wrapper.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: docs
- GitHub Check: testing
- GitHub Check: Summary
🔇 Additional comments (3)
meta/bio/alignoth_report/meta_wrapper.smk (3)
3-14: LGTM!The checkpoint is well-structured and correctly configured to extract variant information.
17-31: LGTM!The rule correctly uses the
--around-vcf-recordparameter as discussed in previous comments, enabling efficient per-variant processing.
59-60: LGTM!The variant counting implementation is efficient and correctly accesses the checkpoint output. The generator expression ensures memory-efficient line counting.
…pers into alignoth-report
… meta-wrapper module import will use master)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
meta/bio/alignoth_report/test/Snakefile (1)
5-6: Path issue already flagged - see previous review comment.This path issue has already been reported in a previous review. The meta_wrapper path needs to be relative to this Snakefile's parent directory to resolve the CI failure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
meta/bio/alignoth_report/test/Snakefile(1 hunks)meta/bio/bowtie2_sambamba/test/Snakefile(1 hunks)meta/bio/bwa_mapping/test/Snakefile(1 hunks)meta/bio/calc_consensus_reads/test/Snakefile(1 hunks)meta/bio/dada2_pe/test/Snakefile(1 hunks)meta/bio/dada2_se/test/Snakefile(1 hunks)meta/bio/gatk_mutect2_calling/test/Snakefile(1 hunks)meta/bio/salmon_tximport/test/Snakefile(1 hunks)meta/bio/star_arriba/test/Snakefile(1 hunks)test_wrappers.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
Files:
test_wrappers.py
🧠 Learnings (17)
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefiletest_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefiletest_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-26T10:49:54.765Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefiletest_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefiletest_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-08-21T08:33:50.878Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:33:50.878Z
Learning: The `snakemake` variable is inserted via a preamble during execution in `wrapper.py` scripts, so it doesn't need to be explicitly defined.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/bowtie2_sambamba/test/Snakefile
📚 Learning: 2024-11-26T10:49:04.406Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:1-10
Timestamp: 2024-11-26T10:49:04.406Z
Learning: In the `snakemake-wrappers` project, it's acceptable to use "master" in the wrapper path instead of pinning to a specific version.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-26T09:16:24.981Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:1-13
Timestamp: 2024-11-26T09:16:24.981Z
Learning: In test `Snakefile`s (e.g., `test/Snakefile`), it's acceptable to use fixed input and output file names instead of wildcards.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-15T18:32:07.612Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:11-12
Timestamp: 2024-11-15T18:32:07.612Z
Learning: In the `snakemake-wrappers` repository, when developing wrappers, it's acceptable to use "master" as the wrapper version in the `wrapper` directive. This ensures that the documentation always reflects the latest stable version of the wrappers.
Applied to files:
meta/bio/salmon_tximport/test/Snakefilemeta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-15T18:31:15.447Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:7-10
Timestamp: 2024-11-15T18:31:15.447Z
Learning: In the Snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.gettempdir()` or changing output paths to temporary directories.
Applied to files:
meta/bio/dada2_se/test/Snakefilemeta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefiletest_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/gatk_mutect2_calling/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-26T12:03:25.328Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/meta.yaml:1-21
Timestamp: 2024-11-26T12:03:25.328Z
Learning: For meta-wrappers in snakemake-wrappers, the `meta.yaml` does not require keys for parameters (`params`), input, or output specifications, and the documentation requirements differ from regular wrappers.
Applied to files:
meta/bio/star_arriba/test/Snakefiletest_wrappers.py
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Applied to files:
meta/bio/star_arriba/test/Snakefilemeta/bio/calc_consensus_reads/test/Snakefilemeta/bio/dada2_pe/test/Snakefiletest_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/bwa_mapping/test/Snakefilemeta/bio/alignoth_report/test/Snakefile
📚 Learning: 2024-11-15T13:48:33.759Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/preprocess-variants/wrapper.py:0-0
Timestamp: 2024-11-15T13:48:33.759Z
Learning: In Snakemake wrappers, security considerations like input sanitization are unnecessary, as the wrappers are under full control of the user.
Applied to files:
meta/bio/dada2_pe/test/Snakefile
📚 Learning: 2024-11-26T14:59:03.678Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.
Applied to files:
test_wrappers.pymeta/bio/bowtie2_sambamba/test/Snakefilemeta/bio/bwa_mapping/test/Snakefile
📚 Learning: 2024-11-15T18:36:04.660Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/call-variants/wrapper.py:15-23
Timestamp: 2024-11-15T18:36:04.660Z
Learning: In the Snakemake wrappers repository, using `shell=True` and redirecting within shell commands is acceptable.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-26T08:30:23.818Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-26T15:01:13.202Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:1-23
Timestamp: 2024-11-26T15:01:13.202Z
Learning: The NGS-bits SampleAncestry wrapper in `bio/ngsbits/sampleancestry/` includes a test Snakefile, sample VCF files, and tests available in the `test/` directory.
Applied to files:
meta/bio/gatk_mutect2_calling/test/Snakefile
📚 Learning: 2024-12-06T14:25:43.922Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3498
File: bio/ngscheckmate/ncm/wrapper.py:68-68
Timestamp: 2024-12-06T14:25:43.922Z
Learning: In the `bio/ngscheckmate/ncm/wrapper.py` file for the NGSCheckMate wrapper, do not suggest adding file existence checks for the list file input, as the files are already required by the wrapper.
Applied to files:
meta/bio/alignoth_report/test/Snakefile
🪛 Ruff (0.14.3)
test_wrappers.py
83-83: Trace found: breakpoint used
(T100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: testing
- GitHub Check: docs
- GitHub Check: Summary
🔇 Additional comments (10)
meta/bio/bowtie2_sambamba/test/Snakefile (1)
8-8: LGTM! Consistent pattern for local wrapper resolution.The switch to a file-based wrapper reference is consistent with the broader changes in this PR and enables local testing of meta-wrappers.
meta/bio/gatk_mutect2_calling/test/Snakefile (1)
6-6: LGTM! Wrapper reference updated correctly.Consistent with the repository-wide pattern of using local file-based wrapper references.
meta/bio/salmon_tximport/test/Snakefile (1)
6-6: LGTM! Follows established pattern.meta/bio/dada2_se/test/Snakefile (1)
6-6: LGTM!meta/bio/bwa_mapping/test/Snakefile (1)
8-8: LGTM!meta/bio/star_arriba/test/Snakefile (1)
8-8: LGTM!test_wrappers.py (2)
45-46: Well-structured meta-wrapper handling.The logic correctly differentiates meta-wrappers from regular wrappers, skipping the wrapper-prefix for meta-wrappers since they define their own versioned wrapper paths.
Also applies to: 89-100
268-278: New test follows established pattern.The
test_alignoth_report_metafunction correctly exercises the new alignoth_report meta-wrapper with appropriate parameters.meta/bio/dada2_pe/test/Snakefile (1)
6-6: The relative path is broken and references a non-existent directory.The change at line 6 has a critical issue: the path
file:../master/meta/bio/dada2_pedoes not exist in the repository. There is nomaster/directory, and the path cannot resolve from the test file location.The actual
meta_wrapper.smkfile is located atmeta/bio/dada2_pe/meta_wrapper.smk, which from the test file location (meta/bio/dada2_pe/test/Snakefile) should be referenced asfile:../meta_wrapper.smk.meta_wrapper: "file:../meta_wrapper.smk"⛔ Skipped due to learnings
Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3499 File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22 Timestamp: 2024-11-26T09:16:39.570Z Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3496 File: bio/mtnucratio/test/Snakefile:2-6 Timestamp: 2024-11-26T08:31:00.099Z Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3501 File: meta/bio/varscan2_snpeff/test/Snakefile:58-71 Timestamp: 2024-11-26T10:49:54.765Z Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3499 File: bio/ngscheckmate/makesnvpattern/test/Snakefile:1-13 Timestamp: 2024-11-26T09:16:24.981Z Learning: In test `Snakefile`s (e.g., `test/Snakefile`), it's acceptable to use fixed input and output file names instead of wildcards.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3478 File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:7-10 Timestamp: 2024-11-15T18:31:15.447Z Learning: In the Snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.gettempdir()` or changing output paths to temporary directories.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3501 File: meta/bio/varscan2_snpeff/test/Snakefile:1-10 Timestamp: 2024-11-26T10:49:04.406Z Learning: In the `snakemake-wrappers` project, it's acceptable to use "master" in the wrapper path instead of pinning to a specific version.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3498 File: bio/ngscheckmate/ncm/wrapper.py:68-68 Timestamp: 2024-12-06T14:25:43.922Z Learning: In the `bio/ngscheckmate/ncm/wrapper.py` file for the NGSCheckMate wrapper, do not suggest adding file existence checks for the list file input, as the files are already required by the wrapper.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3497 File: bio/sexdeterrmine/wrapper.py:23-27 Timestamp: 2024-11-26T08:32:22.480Z Learning: In Snakemake scripts, in-script directory changes do not affect the rest of the pipeline because Snakemake and Shell handle execution context, even when the script stops on error.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3478 File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:11-12 Timestamp: 2024-11-15T18:32:07.612Z Learning: In the `snakemake-wrappers` repository, when developing wrappers, it's acceptable to use "master" as the wrapper version in the `wrapper` directive. This ensures that the documentation always reflects the latest stable version of the wrappers.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3497 File: bio/sexdeterrmine/wrapper.py:23-26 Timestamp: 2024-11-26T08:35:42.140Z Learning: In the `bio/sexdeterrmine/wrapper.py` file (Python), we rely on Samtools to handle input validation for the depth file provided by the user, so additional file existence checks are not necessary.meta/bio/calc_consensus_reads/test/Snakefile (1)
8-8: Thefile:protocol path references a non-existent directory and will fail at runtime.The URL must point to the folder containing the wrapper. The path
file:../master/meta/bio/calc_consensus_readsattempts to navigate to a "master" directory that does not exist in the repository structure. The actual wrapper files (meta_wrapper.smk, environment.yaml, etc.) are located directly in the parent directory from the test Snakefile, not nested under a "master" path.From
meta/bio/calc_consensus_reads/test/, the correct path should be simplyfile:../to reach the wrapper folder.⛔ Skipped due to learnings
Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3499 File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22 Timestamp: 2024-11-26T09:16:39.570Z Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3496 File: bio/mtnucratio/test/Snakefile:2-6 Timestamp: 2024-11-26T08:31:00.099Z Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3501 File: meta/bio/varscan2_snpeff/test/Snakefile:58-71 Timestamp: 2024-11-26T10:49:54.765Z Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3478 File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:7-10 Timestamp: 2024-11-15T18:31:15.447Z Learning: In the Snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.gettempdir()` or changing output paths to temporary directories.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3499 File: bio/ngscheckmate/makesnvpattern/test/Snakefile:1-13 Timestamp: 2024-11-26T09:16:24.981Z Learning: In test `Snakefile`s (e.g., `test/Snakefile`), it's acceptable to use fixed input and output file names instead of wildcards.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3498 File: bio/ngscheckmate/ncm/wrapper.py:68-68 Timestamp: 2024-12-06T14:25:43.922Z Learning: In the `bio/ngscheckmate/ncm/wrapper.py` file for the NGSCheckMate wrapper, do not suggest adding file existence checks for the list file input, as the files are already required by the wrapper.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3501 File: meta/bio/varscan2_snpeff/test/Snakefile:1-10 Timestamp: 2024-11-26T10:49:04.406Z Learning: In the `snakemake-wrappers` project, it's acceptable to use "master" in the wrapper path instead of pinning to a specific version.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3502 File: bio/ngsbits/sampleancestry/wrapper.py:18-23 Timestamp: 2024-11-26T14:59:03.678Z Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3478 File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12 Timestamp: 2024-11-21T10:23:03.427Z Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.Learnt from: johanneskoester Repo: snakemake/snakemake-wrappers PR: 3478 File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:11-12 Timestamp: 2024-11-15T18:32:07.612Z Learning: In the `snakemake-wrappers` repository, when developing wrappers, it's acceptable to use "master" as the wrapper version in the `wrapper` directive. This ensures that the documentation always reflects the latest stable version of the wrappers.Learnt from: tdayris Repo: snakemake/snakemake-wrappers PR: 3497 File: bio/sexdeterrmine/wrapper.py:23-26 Timestamp: 2024-11-26T08:35:42.140Z Learning: In the `bio/sexdeterrmine/wrapper.py` file (Python), we rely on Samtools to handle input validation for the depth file provided by the user, so additional file existence checks are not necessary.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
meta/bio/alignoth_report/meta_wrapper.smk (1)
46-46: Remove or document commented code.The commented output line should either be removed if unused or documented with a comment explaining why it's kept for future reference.
Apply this diff if the line is no longer needed:
htmlindex="index.html", ), - # config = "resources/datavzrd/{sample}.rendered_config.yaml" params:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
meta/bio/alignoth_report/common.smk(1 hunks)meta/bio/alignoth_report/meta_wrapper.smk(1 hunks)
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3115
File: CHANGELOG.md:5-5
Timestamp: 2024-10-08T17:41:54.542Z
Learning: Do not review release-please commits in the Snakemake wrappers repository as they are auto-formatted.
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3115
File: CHANGELOG.md:5-5
Timestamp: 2024-08-14T15:21:37.230Z
Learning: Do not review release-please commits in the Snakemake wrappers repository as they are auto-formatted.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:11-12
Timestamp: 2024-11-15T18:32:07.612Z
Learning: In the `snakemake-wrappers` repository, when developing wrappers, it's acceptable to use "master" as the wrapper version in the `wrapper` directive. This ensures that the documentation always reflects the latest stable version of the wrappers.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
📚 Learning: 2025-10-31T08:33:13.888Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 4678
File: meta/bio/alignoth_report/meta_wrapper.smk:53-56
Timestamp: 2025-10-31T08:33:13.888Z
Learning: In Snakemake input functions (e.g., functions used in `input:` sections), when returning paths that should contain wildcards, it's correct to return strings with literal wildcard placeholders (e.g., `f"{results_dir}/alignoth/{{sample}}/{i}/"`). Snakemake applies wildcard substitution on the returned values from these functions, so the wildcards don't need to be resolved within the function itself using `wildcards.sample`.
Applied to files:
meta/bio/alignoth_report/common.smkmeta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T12:03:25.328Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/meta.yaml:1-21
Timestamp: 2024-11-26T12:03:25.328Z
Learning: For meta-wrappers in snakemake-wrappers, the `meta.yaml` does not require keys for parameters (`params`), input, or output specifications, and the documentation requirements differ from regular wrappers.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-08-21T08:33:50.878Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:33:50.878Z
Learning: The `snakemake` variable is inserted via a preamble during execution in `wrapper.py` scripts, so it doesn't need to be explicitly defined.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-15T18:32:07.612Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:11-12
Timestamp: 2024-11-15T18:32:07.612Z
Learning: In the `snakemake-wrappers` repository, when developing wrappers, it's acceptable to use "master" as the wrapper version in the `wrapper` directive. This ensures that the documentation always reflects the latest stable version of the wrappers.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-15T13:48:33.759Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/preprocess-variants/wrapper.py:0-0
Timestamp: 2024-11-15T13:48:33.759Z
Learning: In Snakemake wrappers, security considerations like input sanitization are unnecessary, as the wrappers are under full control of the user.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T08:30:23.818Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-12-06T14:25:43.922Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3498
File: bio/ngscheckmate/ncm/wrapper.py:68-68
Timestamp: 2024-12-06T14:25:43.922Z
Learning: In the `bio/ngscheckmate/ncm/wrapper.py` file for the NGSCheckMate wrapper, do not suggest adding file existence checks for the list file input, as the files are already required by the wrapper.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2025-10-31T14:56:28.280Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 4665
File: meta/bio/salmon_tximport/meta_wrapper.smk:91-125
Timestamp: 2025-10-31T14:56:28.280Z
Learning: In Snakemake wrappers, pathvars only expand to strings, not lists or other data structures. Sample lists should be provided via config["samples"] rather than through pathvars, following standard Snakemake configuration patterns.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T14:59:03.678Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2025-06-15T07:43:03.263Z
Learnt from: tedil
Repo: snakemake/snakemake-wrappers PR: 4274
File: bio/mehari/annotate-seqvars/wrapper.py:11-19
Timestamp: 2025-06-15T07:43:03.263Z
Learning: In Snakemake wrappers, a common pattern is to have flag variables that are either empty strings "" or the actual flag strings (e.g., "--keep-intergenic") for direct interpolation in shell commands. This avoids conditionals in the shell expression and keeps the command construction clean.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T09:16:24.981Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:1-13
Timestamp: 2024-11-26T09:16:24.981Z
Learning: In test `Snakefile`s (e.g., `test/Snakefile`), it's acceptable to use fixed input and output file names instead of wildcards.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-15T18:31:15.447Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:7-10
Timestamp: 2024-11-15T18:31:15.447Z
Learning: In the Snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.gettempdir()` or changing output paths to temporary directories.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T10:49:54.765Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-15T18:36:04.660Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/call-variants/wrapper.py:15-23
Timestamp: 2024-11-15T18:36:04.660Z
Learning: In the Snakemake wrappers repository, using `shell=True` and redirecting within shell commands is acceptable.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
📚 Learning: 2024-11-26T15:01:13.202Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:1-23
Timestamp: 2024-11-26T15:01:13.202Z
Learning: The NGS-bits SampleAncestry wrapper in `bio/ngsbits/sampleancestry/` includes a test Snakefile, sample VCF files, and tests available in the `test/` directory.
Applied to files:
meta/bio/alignoth_report/meta_wrapper.smk
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: testing
- GitHub Check: docs
- GitHub Check: Summary
🔇 Additional comments (2)
meta/bio/alignoth_report/meta_wrapper.smk (2)
3-14: LGTM: Checkpoint structure is correct.The vembrane_table checkpoint is properly configured with appropriate input, output, logging, and wrapper reference.
17-33: LGTM: alignoth rule structure is correct.The rule properly declares all required inputs, produces a directory output, and constructs the extra parameters from the wildcard index. The lambda signature includes
inputparameter which follows Snakemake conventions even when unused.
meta/bio/alignoth_report/common.smk
Outdated
Comment on lines
8
to
9
| def count_variants(wildcards): | ||
| return sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 |
Contributor
There was a problem hiding this comment.
Fix resource leak and convert to lambda per linting requirement.
The file handle is never closed, causing a resource leak. Additionally, the linting failure indicates that one-liner functions should be defined as lambda expressions.
Apply this diff:
-def count_variants(wildcards):
- return sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1
+count_variants = lambda wildcards: sum(1 for line in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0])) - 1Note: While this addresses the linting requirement, Python's garbage collector will eventually close the file. For more explicit resource management, consider wrapping in a proper function with context manager if the linting rules allow it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def count_variants(wildcards): | |
| return sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 | |
| count_variants = lambda wildcards: sum(1 for line in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0])) - 1 |
Suggested change
| def count_variants(wildcards): | |
| return sum(1 for _ in open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0], "r")) - 1 | |
| def count_variants(wildcards): | |
| with open(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0]) as f: | |
| return sum(1 for _ in f) - 1 |
🤖 Prompt for AI Agents
In meta/bio/alignoth_report/common.smk around lines 8-9, replace the def
count_variants(...) one-liner with a lambda that does not leave a file handle
open; e.g. assign count_variants = lambda wildcards:
len(Path(checkpoints.vembrane_table.get(sample=wildcards.sample).output[0]).read_text().splitlines())
- 1 and ensure pathlib.Path is imported at the top of the file if not already
present.
johanneskoester
changed the title
…pers into alignoth-report
Contributor
Author
Contributor
Author
|
@johanneskoester thanks for the fixes regarding the wrapper paths! Can we merge this despite the snpeff issue? The linting is currently failing as well caused by Is this a bug in snakemake or do I miss something? |
Contributor
|
We have to wait for this final PR now: snakemake/snakemake#3829 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test_wrappers.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
Files:
test_wrappers.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:1-10
Timestamp: 2024-11-26T10:49:04.406Z
Learning: In the `snakemake-wrappers` project, it's acceptable to use "master" in the wrapper path instead of pinning to a specific version.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:11-12
Timestamp: 2024-11-15T18:32:07.612Z
Learning: In the `snakemake-wrappers` repository, when developing wrappers, it's acceptable to use "master" as the wrapper version in the `wrapper` directive. This ensures that the documentation always reflects the latest stable version of the wrappers.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/meta.yaml:1-21
Timestamp: 2024-11-26T12:03:25.328Z
Learning: For meta-wrappers in snakemake-wrappers, the `meta.yaml` does not require keys for parameters (`params`), input, or output specifications, and the documentation requirements differ from regular wrappers.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
📚 Learning: 2024-11-26T10:49:54.765Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-26T14:59:03.678Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-15T18:31:15.447Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:7-10
Timestamp: 2024-11-15T18:31:15.447Z
Learning: In the Snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.gettempdir()` or changing output paths to temporary directories.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-15T18:36:04.660Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/call-variants/wrapper.py:15-23
Timestamp: 2024-11-15T18:36:04.660Z
Learning: In the Snakemake wrappers repository, using `shell=True` and redirecting within shell commands is acceptable.
Applied to files:
test_wrappers.py
📚 Learning: 2024-11-26T08:30:23.818Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
Applied to files:
test_wrappers.py
📚 Learning: 2024-08-21T08:30:42.757Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:30:42.757Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.
Applied to files:
test_wrappers.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: docs
- GitHub Check: testing
- GitHub Check: Summary
🔇 Additional comments (3)
test_wrappers.py (3)
2-2: LGTM! Clear setup for meta-wrapper detection.The Path import and meta-wrapper detection logic are well-structured. Using
Pathfor filesystem operations and a simple string prefix check for classification is appropriate.Also applies to: 45-47
97-108: LGTM! Command construction correctly handles meta-wrappers.The conditional logic appropriately adds
--wrapper-prefixonly for non-meta wrappers, allowing meta-wrappers to define their own specific wrapper versions as intended.
276-286: LGTM! Test function follows established patterns.The new test for the alignoth_report meta-wrapper is consistent with other meta-wrapper test functions in the file.
johanneskoester
pushed a commit
that referenced
this pull request
Nov 12, 2025
🤖 I have created a release *beep* *boop* --- ## [8.0.0](v7.9.1...v8.0.0) (2025-11-11) ### ⚠ BREAKING CHANGES * Pin wrapper versions in meta-wrappers; add alignoth_report meta-wrapper ([#4678](#4678)) ### Features * Pin wrapper versions in meta-wrappers; add alignoth_report meta-wrapper ([#4678](#4678)) ([df95006](df95006)) ### Bug Fixes * downgrade bcftools in vep environment ([#4713](#4713)) ([f6f3104](f6f3104)) ### Performance Improvements * autobump bio/delly/environment.yaml ([#4701](#4701)) ([ac0eccc](ac0eccc)) * autobump bio/diamond/blastp/environment.yaml ([#4696](#4696)) ([69ca46b](69ca46b)) * autobump bio/diamond/blastx/environment.yaml ([#4697](#4697)) ([a78f857](a78f857)) * autobump bio/diamond/makedb/environment.yaml ([#4700](#4700)) ([cda63aa](cda63aa)) * autobump bio/gdc-api/bam-slicing/environment.yaml ([#4702](#4702)) ([4442ff1](4442ff1)) * autobump bio/mehari/download-clinvar-db/environment.yaml ([#4704](#4704)) ([60b7a96](60b7a96)) * autobump bio/mehari/download-transcript-db/environment.yaml ([#4703](#4703)) ([c9e468b](c9e468b)) * autobump bio/picard/collectinsertsizemetrics/environment.yaml ([#4706](#4706)) ([cc3bf0e](cc3bf0e)) * autobump utils/datavzrd/environment.yaml ([#4699](#4699)) ([0b40b95](0b40b95)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Nov 14, 2025
7 tasks
7 tasks
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.
QC
snakemake-wrappers.While the contributions guidelines are more extensive, please particularly ensure that:
test.pywas updated to call any added or updated example rules in aSnakefileinput:andoutput:file paths in the rules can be chosen arbitrarilyinput:oroutput:)tempfile.gettempdir()points tometa.yamlcontains a link to the documentation of the respective tool or command underurl:Summary by CodeRabbit
New Features
Improvements
Tests
Chores