feat: add wrapper for rbt vcf_fix_iupac_alleles #1209
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
johanneskoester
requested changes
Apr 28, 2023
|
|
||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| shell( | ||
| "(rbt vcf-fix-iupac-alleles {extra} < {snakemake.input[0]} | bcftools sort --temp-dir {tmpdir} {bcftools_opts}) {log}" |
There was a problem hiding this comment.
Why the sort? rbt vcf-fix-iupac-alleles does not change the order of the records.
There was a problem hiding this comment.
The main idea was to allow for automatic output on any format (bcf, vcf, vcf.gz).
Since I was using it to standardize resources (known variation from ensembl), I thought that sort would be a nice-to-have (just to be sure the output was sorted) and the extra runtime would be ok since it would only be run once.
But I can also remove it (or switch to view), if you prefer to keep the wrapper more general.
There was a problem hiding this comment.
Ping @johanneskoester
|
This PR was marked as stale because it has been open for 6 months with no activity. |
|
This PR was marked as stale because it has been open for 6 months with no activity. |
|
This PR was marked as stale because it has been open for 6 months with no activity. |
📝 WalkthroughWalkthroughNew support for the Changes
Sequence Diagram(s)sequenceDiagram
participant Snakemake
participant Wrapper (wrapper.py)
participant rbt vcf-fix-iupac-alleles
participant bcftools sort
Snakemake->>Wrapper (wrapper.py): Run rule with input VCF/BCF
Wrapper (wrapper.py)->>rbt vcf-fix-iupac-alleles: Process input VCF
rbt vcf-fix-iupac-alleles-->>Wrapper (wrapper.py): Output VCF stream
Wrapper (wrapper.py)->>bcftools sort: Sort output with tempdir and options
bcftools sort-->>Wrapper (wrapper.py): Sorted VCF/BCF
Wrapper (wrapper.py)-->>Snakemake: Write output file, log execution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This PR was marked as stale because it has been open for 6 months with no activity. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
bio/rbt/vcf_fix_iupac_alleles/wrapper.py (1)
15-18: Remove the superfluousbcftools sortstage – it adds runtime without changing output order
rbt vcf-fix-iupac-allelesdoes not alter record coordinates, so an additional sort is redundant and just burns I/O and CPU. Unless you have a proven downstream requirement, consider piping the result directly tobcftools view(or nothing at all) and let the usualbcftools_optstake care of compression / output-type selection.- "(rbt vcf-fix-iupac-alleles {extra} < {snakemake.input[0]} | bcftools sort --temp-dir {tmpdir} {bcftools_opts}) {log}" + "(rbt vcf-fix-iupac-alleles {extra} < {snakemake.input[0]} | bcftools view {bcftools_opts}) {log}"This trims one full pass over the data and removes the temporary directory dependency while keeping identical output semantics.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bio/rbt/vcf_fix_iupac_alleles/test/homo_sapiens-chrMT.vcf.gzis excluded by!**/*.gz
📒 Files selected for processing (5)
bio/rbt/vcf_fix_iupac_alleles/environment.yaml(1 hunks)bio/rbt/vcf_fix_iupac_alleles/meta.yaml(1 hunks)bio/rbt/vcf_fix_iupac_alleles/test/Snakefile(1 hunks)bio/rbt/vcf_fix_iupac_alleles/wrapper.py(1 hunks)test.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
bio/rbt/vcf_fix_iupac_alleles/wrapper.pytest.py
**/wrapper.py
⚙️ CodeRabbit Configuration File
Do not complain about use of undefined variable called
snakemake.
Files:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
🧠 Learnings (25)
📓 Common learnings
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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
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.
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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.
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#3499
File: bio/ngscheckmate/makesnvpattern/wrapper.py:18-24
Timestamp: 2024-11-26T09:08:06.041Z
Learning: In Snakemake wrappers, input file validation is managed by Snakemake, so manual validation in the wrapper code is unnecessary.
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.
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3478
File: bio/varlociraptor/control-fdr/test/Snakefile:1-55
Timestamp: 2024-11-21T10:49:23.499Z
Learning: The `varlociraptor control-fdr` wrapper is tested upstream.
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3302
File: bio/vg/giraffe/test/Snakefile:26-29
Timestamp: 2024-11-22T07:35:06.812Z
Learning: For the `vg_giraffe_map` rule in `bio/vg/giraffe/test/Snakefile`, adding explicit parameter validation functions like `validate_sort_params` is considered unnecessary and may be overkill.
Learnt from: dlaehnemann
PR: snakemake/snakemake-wrappers#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
PR: snakemake/snakemake-wrappers#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
PR: snakemake/snakemake-wrappers#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.
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.
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.
📚 Learning: in snakemake-wrappers repository, environment.yaml files should follow these conventions: 1. use whi...
Learnt from: dlaehnemann
PR: snakemake/snakemake-wrappers#0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See full guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/environment.yaml
📚 Learning: in snakemake-wrappers repository, environment.yaml files should follow these conventions: 1. use whi...
Learnt from: dlaehnemann
PR: snakemake/snakemake-wrappers#0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/environment.yaml
📚 Learning: in the nanosim bioconda recipe, dependencies are carefully managed with specific version pins (e.g.,...
Learnt from: dlaehnemann
PR: snakemake/snakemake-wrappers#3648
File: bio/nanosim/simulator/environment.yaml:6-6
Timestamp: 2025-02-11T12:24:22.592Z
Learning: In the nanosim bioconda recipe, dependencies are carefully managed with specific version pins (e.g., scikit-learn ~=0.23.2) to ensure compatibility with pre-trained models. These dependencies don't need to be explicitly added to environment.yaml files when the main package is listed as a dependency, as they are handled through the bioconda recipe system.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/environment.yaml
📚 Learning: in the snakemake-wrapper repository, conda dependency version pinning in environment.yaml files uses...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#4159
File: bio/pyfaidx/environment.yaml:6-6
Timestamp: 2025-06-02T07:56:35.854Z
Learning: In the Snakemake-wrapper repository, conda dependency version pinning in environment.yaml files uses spaces around the equals sign (e.g., `- pyfaidx =0.8.1.4`) as the established coding standard, even though conda itself doesn't require the spaces.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/environment.yaml
📚 Learning: for bioconda packages like nanosim, dependencies like scikit-learn are managed through the bioconda ...
Learnt from: dlaehnemann
PR: snakemake/snakemake-wrappers#3648
File: bio/nanosim/simulator/environment.yaml:6-6
Timestamp: 2025-02-11T12:24:22.592Z
Learning: For bioconda packages like nanosim, dependencies like scikit-learn are managed through the bioconda recipe and don't need to be explicitly added to the environment.yaml file when the main package is listed as a dependency.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/environment.yaml
📚 Learning: in snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of too...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/environment.yamlbio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in test files for snakemake wrappers, such as `bio/mtnucratio/test/snakefile`, hard-coded input and ...
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:
bio/rbt/vcf_fix_iupac_alleles/environment.yamlbio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.pytest.py
📚 Learning: in the `snakemake-wrappers` repository, when writing test `snakefile`s (e.g., `bio/ngscheckmate/make...
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:
bio/rbt/vcf_fix_iupac_alleles/environment.yamlbio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.pytest.py
📚 Learning: the ngs-bits sampleancestry wrapper in `bio/ngsbits/sampleancestry/` includes a test snakefile, samp...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/environment.yamlbio/rbt/vcf_fix_iupac_alleles/meta.yamlbio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.pytest.py
📚 Learning: for the `vg_giraffe_map` rule in `bio/vg/giraffe/test/snakefile`, adding explicit parameter validati...
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3302
File: bio/vg/giraffe/test/Snakefile:26-29
Timestamp: 2024-11-22T07:35:06.812Z
Learning: For the `vg_giraffe_map` rule in `bio/vg/giraffe/test/Snakefile`, adding explicit parameter validation functions like `validate_sort_params` is considered unnecessary and may be overkill.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in test `snakefile`s (e.g., `test/snakefile`), it's acceptable to use fixed input and output file na...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.pytest.py
📚 Learning: in test snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified path...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.pytest.py
📚 Learning: in the snakemake wrappers project, avoid suggesting extensive error handling or temporary file manag...
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:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: do not review release-please commits in the snakemake wrappers repository as they are auto-formatted...
Learnt from: dlaehnemann
PR: snakemake/snakemake-wrappers#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.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefile
📚 Learning: using generic output filenames like "genome.fasta" is acceptable in the `snakemake-wrappers` project...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#3501
File: meta/bio/varscan2_snpeff/test/Snakefile:1-10
Timestamp: 2024-11-26T10:49:04.406Z
Learning: Using generic output filenames like "genome.fasta" is acceptable in the `snakemake-wrappers` project.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in the `bio/sexdeterrmine/wrapper.py` file (python), we rely on samtools to handle input validation ...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in the `bio/ngscheckmate/ncm/wrapper.py` file for the ngscheckmate wrapper, do not suggest adding fi...
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:
bio/rbt/vcf_fix_iupac_alleles/test/Snakefilebio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`....
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:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in snakemake wrapper scripts, snakemake validates input and output paths, so explicit shell quoting ...
Learnt from: tdayris
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in the snakemake-wrappers repository, it is acceptable to use f-strings with `shell=true` in wrapper...
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-15T13:44:18.810Z
Learning: In the snakemake-wrappers repository, it is acceptable to use f-strings with `shell=True` in wrapper scripts (e.g., `wrapper.py` files), as these wrappers are under full control of the user. Potential command injection vulnerabilities are not considered an issue in this context.
Applied to files:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in the snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.get...
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in the snakemake wrappers repository, using `shell=true` and redirecting within shell commands is ac...
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in snakemake wrappers, a common pattern is to have flag variables that are either empty strings "" o...
Learnt from: tedil
PR: snakemake/snakemake-wrappers#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:
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
📚 Learning: in example snakefiles, using hardcoded sample names is acceptable....
Learnt from: johanneskoester
PR: snakemake/snakemake-wrappers#3478
File: bio/varlociraptor/call-variants/test/Snakefile:29-47
Timestamp: 2024-11-21T10:50:09.006Z
Learning: In example Snakefiles, using hardcoded sample names is acceptable.
Applied to files:
test.py
🪛 Ruff (0.12.2)
bio/rbt/vcf_fix_iupac_alleles/wrapper.py
10-10: Undefined name snakemake
(F821)
11-11: Undefined name snakemake
(F821)
12-12: Undefined name snakemake
(F821)
🔇 Additional comments (4)
bio/rbt/vcf_fix_iupac_alleles/meta.yaml (1)
1-10: Meta looks good – just confirm naming consistencyEverything is well-formed. Double-check that other
rbtwrappers use the same dash-separated style (rbt vcf-fix-…) so the naming stays consistent across the repo.test.py (2)
6124-6128: LGTM – renamed test executes existing wrapper correctly
The new name follows snake_case and keeps the invocation logic unchanged. No issues spotted.
6132-6142: Consider looping over the two output formats to avoid duplication
Both calls differ only in the expected output extension. Collapsing them into one param-driven loop trims repetitive code and reduces test-runtime overhead.-@skip_if_not_modified -def test_rbt_vcf_fix_iupac_alleles(): - run( - "bio/rbt/vcf_fix_iupac_alleles", - ["snakemake", "--cores", "1", "--use-conda", "-F", "results/homo_sapiens-chrMT.vcf.gz"], - ) - - run( - "bio/rbt/vcf_fix_iupac_alleles", - ["snakemake", "--cores", "1", "--use-conda", "-F", "results/homo_sapiens-chrMT.bcf"], - ) +@skip_if_not_modified +def test_rbt_vcf_fix_iupac_alleles(): + for ext in (".vcf.gz", ".bcf"): + run( + "bio/rbt/vcf_fix_iupac_alleles", + [ + "snakemake", + "--cores", + "1", + "--use-conda", + "-F", + f"results/homo_sapiens-chrMT{ext}", + ], + )[ suggest_optional_refactor ]
bio/rbt/vcf_fix_iupac_alleles/test/Snakefile (1)
2-23: Test rules look goodThe VCF and BCF targets exercise both output formats, paths are hard-coded as expected for wrapper tests, and resource limits are reasonable.
Comment on lines
+6
to
+8
| - rust-bio-tools =0.42.0 | ||
| - bcftools =1.17 | ||
| - snakemake-wrapper-utils =0.5.3 |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Unpin secondary dependencies; keep only the main tool fixed
Wrapper guidelines recommend pinning only the primary software (rust-bio-tools) and letting Bioconda resolve compatible versions for helper tools. Exact pins for bcftools and snakemake-wrapper-utils increase the risk of conflicts with other wrappers and make future updates harder.
- rust-bio-tools =0.42.0
- - bcftools =1.17
- - snakemake-wrapper-utils =0.5.3
+ - bcftools
+ - snakemake-wrapper-utils(Spacing around the = is already correct – nice job).
Unpin secondary dependencies in environment.yaml
Only the primary tool (rust-bio-tools) should be version‐pinned; helper packages (bcftools, snakemake-wrapper-utils) can be left unpinned so Bioconda can resolve compatible versions.
Please update in bio/rbt/vcf_fix_iupac_alleles/environment.yaml (lines 6–8):
- rust-bio-tools =0.42.0
- - bcftools =1.17
- - snakemake-wrapper-utils =0.5.3
+ - bcftools
+ - snakemake-wrapper-utils📝 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
| - rust-bio-tools =0.42.0 | |
| - bcftools =1.17 | |
| - snakemake-wrapper-utils =0.5.3 | |
| - rust-bio-tools =0.42.0 | |
| - bcftools | |
| - snakemake-wrapper-utils |
🤖 Prompt for AI Agents
In bio/rbt/vcf_fix_iupac_alleles/environment.yaml lines 6 to 8, unpin the
versions of the secondary dependencies bcftools and snakemake-wrapper-utils by
removing the version numbers (=1.17 and =0.5.3) so that only rust-bio-tools
remains version-pinned. This allows Bioconda to resolve compatible versions for
the helper packages.
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.
Description
QC
For all wrappers added by this PR,
input:andoutput:file paths in the resulting rule can be changed arbitrarily,threads: xstatement withxbeing a reasonable default,map_readsfor a step that maps reads),environment.yamlspecifications follow the respective best practices,input:oroutput:),Snakefiles and their entries are explained via comments (input:/output:/params:etc.),stderrand/orstdoutare logged correctly (log:), depending on the wrapped tool,tempfile.gettempdir()points to (see here; this also means that using any Pythontempfiledefault behavior works),meta.yamlcontains a link to the documentation of the respective tool or command,Snakefiles pass the linting (snakemake --lint),Snakefiles are formatted with snakefmt,Summary by CodeRabbit
New Features
Tests