Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Snakemake wrapper for Changes
Sequence DiagramsequenceDiagram
participant User
participant Snakemake
participant Wrapper as Wrapper (wrapper.py)
participant Bcftools as Bcftools (plugin fixploidy)
participant FS as FileSystem
User->>Snakemake: run test_bcftools_fixploidy (--cores 3, --use-conda)
Snakemake->>Wrapper: invoke with input=a.bcf, output=a.fixed.bcf, params.extra
Wrapper->>Wrapper: get_bcftools_opts(parse_ref=False, parse_memory=False)
Wrapper->>Wrapper: read params.extra and build log flags
Wrapper->>Bcftools: execute `bcftools plugin fixploidy {opts} {extra} {input}`
Bcftools->>FS: read input and write output stream
Bcftools-->>Wrapper: exit status / logs
Wrapper-->>Snakemake: return status and logs
Snakemake-->>User: report results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bio/bcftools/fixploidy/wrapper.py`:
- Around line 17-21: The code inconsistently accesses inputs: it assigns incall
from snakemake.input[0] but checks snakemake.input["call"]; update the logic to
use the same named input key for both operations (e.g., read incall from
snakemake.input["call"] before testing its extension and potentially wrapping it
with "< <(bcftools view {})"), and adjust min_threads and incall only based on
that named entry to ensure the extension test and the file used match.
- Around line 23-29: The code initializes outcall from snakemake.output[0] but
then checks snakemake.output["call"], causing inconsistent access; change the
initialization to use the named key (set outcall = snakemake.output["call"]) so
both the assignment and the endswith check use snakemake.output["call"] (keep
the rest of the logic using bcftools_opts and min_threads unchanged), and ensure
the named key "call" is present in snakemake.output before use.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 672a3cb6-ed63-4ca5-b972-78552cc1033e
📒 Files selected for processing (3)
bio/bcftools/fixploidy/meta.yamlbio/bcftools/fixploidy/test/Snakefilebio/bcftools/fixploidy/wrapper.py
✅ Files skipped from review due to trivial changes (2)
- bio/bcftools/fixploidy/meta.yaml
- bio/bcftools/fixploidy/test/Snakefile
|
But doesn't |
Co-authored-by: Filipe G. Vieira <1151762+fgvieira@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bio/bcftools/fixploidy/wrapper.py`:
- Line 15: The bcftools plugin call in wrapper.py places the input file after
`{extra}`, which can break plugin option parsing; update the shell invocation
(the shell(...) call invoking "bcftools plugin fixploidy") to move
`{snakemake.input[0]}` immediately after `{bcftools_opts}` and before `{extra}`
so that the input file is passed before the `--`/plugin options (keep `{log}` at
the end).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 437cd957-28b4-476d-b343-f2780458408f
📒 Files selected for processing (1)
bio/bcftools/fixploidy/wrapper.py
| extra = snakemake.params.get("extra", "") | ||
| log = snakemake.log_fmt_shell(stdout=True, stderr=True) | ||
|
|
||
| shell("bcftools plugin fixploidy {bcftools_opts} {extra} {snakemake.input[0]} {log}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n bio/bcftools/fixploidy/wrapper.pyRepository: snakemake/snakemake-wrappers
Length of output: 692
🏁 Script executed:
find bio/bcftools/fixploidy -type fRepository: snakemake/snakemake-wrappers
Length of output: 268
🏁 Script executed:
# Search for bcftools fixploidy usage patterns in the repository
rg "fixploidy" -A 5 -B 5Repository: snakemake/snakemake-wrappers
Length of output: 2153
🏁 Script executed:
cat -n bio/bcftools/fixploidy/test/SnakefileRepository: snakemake/snakemake-wrappers
Length of output: 394
🏁 Script executed:
# Search for other bcftools plugin wrappers to see argument ordering patterns
find bio/bcftools -name "wrapper.py" -type f | head -10 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Repository: snakemake/snakemake-wrappers
Length of output: 7360
🏁 Script executed:
# Check bcftools documentation for fixploidy plugin argument parsing
web_search "bcftools fixploidy plugin argument parsing order"Repository: snakemake/snakemake-wrappers
Length of output: 122
🌐 Web query:
bcftools fixploidy plugin documentation argument parsing
💡 Result:
The bcftools +fixploidy plugin uses standard C getopt_long for argument parsing in its init function. Plugin options (after the "--" separator) are parsed using a static struct option long_options[] defining: - {"default-ploidy",1,0,'d'} - {"force-ploidy",1,0,'f'} - {"ploidy",1,0,'p'} - {"samples",1,0,'s'} Short options: -d, -f, -p, -s. The getopt_long loop processes these: case 'd': default_ploidy = strtod(optarg,&tmp); case 'f': force_ploidy = strtod(optarg,&tmp); case 'p': ploidy_fname = strdup(optarg); case 's': sex_fname = strdup(optarg); It initializes ploidy with defaults for chrX PAR regions if no -p file given, reads samples/sex from -s file, maps samples to sex, and sets sex-to-ploidy mapping. To view full usage: bcftools +fixploidy -h Source code: https://github.com/samtools/bcftools/blob/develop/plugins/fixploidy.c
Citations:
- 1: https://github.com/samtools/bcftools/blob/develop/plugins/fixploidy.c
- 2: https://samtools.github.io/bcftools/howtos/plugin.api.html
🌐 Web query:
bcftools plugin command line syntax "--" separator
💡 Result:
In BCFtools, the "--" separator is used in plugin command lines to distinguish between general/common options (shared across many bcftools commands, such as -o, -O, -r) that come before it, and plugin-specific options that come after it. The general syntax for running a plugin is: bcftools +plugin_name [general options] input.vcf -- [plugin options] Examples from official documentation: bcftools +fill-tags in.bcf -Ob -o out.bcf -- -t AN,AC bcftools +split-vep file.vcf.gz -- -a BCSQ bcftools +tag2tag in.vcf -- --LXX-to-XX -s 3 Some plugins do not accept general options and their usage is bcftools +plugin_name [input] [plugin options], without the "--" separator. Always check the plugin's usage with bcftools +plugin_name -h. To use plugins, set BCFTOOLS_PLUGINS=/path/to/bcftools/plugins or use full path like bcftools +/path/to/plugin.so.
Citations:
- 1: https://samtools.github.io/bcftools/howtos/plugin.split-vep.html
- 2: https://samtools.github.io/bcftools/howtos/plugin.fill-tags.html
- 3: bcftools plugin fill-tags does not work with "-" to read from stdin. samtools/bcftools#1165
- 4: https://samtools.github.io/bcftools/howtos/plugin.tag2tag.html
🏁 Script executed:
# Search for actual usage examples of bcftools plugin fixploidy in the codebase
rg "bcftools.*fixploidy" -A 3 -B 3Repository: snakemake/snakemake-wrappers
Length of output: 1627
🏁 Script executed:
# Check bcftools documentation examples and common patterns
web_search "bcftools +fixploidy usage example input file position"Repository: snakemake/snakemake-wrappers
Length of output: 122
🌐 Web query:
bcftools +fixploidy usage example input file position argument order
💡 Result:
The standard usage of bcftools +fixploidy follows the general bcftools plugin syntax: bcftools +fixploidy [general options] [input.vcf[.gz]] [-- [plugin options]]. Input file position: The VCF/BCF input file is provided as a positional argument after general options (like -O u for uncompressed BCF output) and before the -- separator. If omitted, it reads from standard input (stdin). Examples from source code and usage: - bcftools +fixploidy in.vcf -- -s samples.txt Here, "in.vcf" is the input file. - bcftools +fixploidy haploid.vcf -- -f 2 > diploid.vcf Here, "haploid.vcf" is the input file, -f 2 forces diploid ploidy for all sites. General syntax template: bcftools +fixploidy -Ou -o output.bcf input.vcf -- -s samples.txt -p ploidy.txt Plugin options (after --): - -d, --default-ploidy [default: 2] - -f, --force-ploidy Ignore -p, set same ploidy everywhere - -p, --ploidy CHROM FROM TO SEX PLOIDY - -s, --sex NAME SEX (M/F) - -t, --tags [GT] Ploidy file format example: X 1 60000 M 1 X 2699521 154931043 M 1 Y 1 59373566 M 1 Y 1 59373566 F 0 Sex file example: sampleName1 M
Citations:
- 1: https://github.com/samtools/bcftools/blob/develop/plugins/fixploidy.c
- 2: https://www.biostars.org/p/250212/
Move input file before {extra} to follow bcftools plugin argument syntax.
The bcftools fixploidy plugin requires input files to be positioned before the -- separator and plugin-specific options. If {extra} contains plugin options (e.g., -- -s samples.txt), the current order places the input file after these options, causing incorrect parsing.
Suggested fix
-shell("bcftools plugin fixploidy {bcftools_opts} {extra} {snakemake.input[0]} {log}")
+shell("bcftools plugin fixploidy {bcftools_opts} {snakemake.input[0]} {extra} {log}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bio/bcftools/fixploidy/wrapper.py` at line 15, The bcftools plugin call in
wrapper.py places the input file after `{extra}`, which can break plugin option
parsing; update the shell invocation (the shell(...) call invoking "bcftools
plugin fixploidy") to move `{snakemake.input[0]}` immediately after
`{bcftools_opts}` and before `{extra}` so that the input file is passed before
the `--`/plugin options (keep `{log}` at the end).
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| extra = snakemake.params.get("extra", "") | ||
| log = snakemake.log_fmt_shell(stdout=True, stderr=True) | ||
|
|
||
| shell("bcftools plugin fixploidy {bcftools_opts} {extra} {snakemake.input[0]} {log}") |
There was a problem hiding this comment.
Is this what it is suggesting?
| shell("bcftools plugin fixploidy {bcftools_opts} {extra} {snakemake.input[0]} {log}") | |
| shell("bcftools plugin fixploidy {bcftools_opts} {snakemake.input[0]} -- {extra} {log}") |
2 participants
This PR adds a wrapper for BCFTools' plugin
fixploidy.This plugin does not handle compression by itself, and therefore needs to be wrapped with
bcftools viewcommands to handle gzipped VCF and BCF files.BCFTools' snakemake helper function parameters are forewarded to
bcftools viewcompression, not the plugin itself. The plugin does not have any of the common IO options present in BCFTools' main tools.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