fix: bug with -O being used for gap opening penalty and samtools output directory #6
Conversation
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
|
Clone of the PR snakemake/snakemake-wrappers#1450 |
![[development] Korbit - AI Mentor](https://avatars.githubusercontent.com/in/334508?s=40&u=8e7ea115878aacba56242d7ac87807ea98eac709&v=4){kind=avatar}
|
👋 I'm here to help you review your pull request. When you're ready for me to perform a review, you can comment anywhere on this pull request with this command: As a reminder, here are some helpful tips on how we can collaborate together:
|
|
My review is in progress 📖 - I will have feedback for you in a few minutes! |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Review Summary by Korbit AI
Code Execution Comments
- Ensure 'gap_opening' defaults to "0" to avoid invalid command line arguments when unspecified in snakemake.
Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Current Korbit Configuration
General Settings
Setting Value Review Schedule Automatic excluding drafts Max Issue Count 10 Automatic PR Descriptions ✅ Issue Categories
Category Enabled Naming ✅ Database Operations ✅ Documentation ✅ Logging ✅ Error Handling ✅ Systems and Environment ✅ Objects and Data Structures ✅ Tests ❌ Readability and Maintainability ✅ Asynchronous Processing ✅ Design Patterns ✅ Third-Party Libraries ✅ Performance ✅ Security ✅ Functionality ✅ Feedback and Support
| @@ -15,6 +15,7 @@ | |||
| log = snakemake.log_fmt_shell(stdout=False, stderr=True) | |||
| sort = snakemake.params.get("sorting", "none") | |||
| sort_extra = snakemake.params.get("sort_extra", "") | |||
| gap_opening = snakemake.params.get("gap_opening", "") | |||
There was a problem hiding this comment.
The code retrieves the 'gap_opening' parameter from snakemake using snakemake.params.get("gap_opening", ""). However, if this parameter is not provided in the snakemake configuration, it will default to an empty string. This can lead to invalid command line arguments being passed to minimap2 if the empty string is used as the value for the '-O' option.
To handle this case more robustly, consider providing a default value for the 'gap_opening' parameter. For example, you can modify the line to: gap_opening = snakemake.params.get("gap_opening", "0"). This way, if the parameter is not specified, it will default to "0" instead of an empty string, ensuring a valid value is always passed to minimap2.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
Description
Gap penalty of minimap2 is done via "-O" option and the very same option also exists for samtools, but to indicate output directory. Using "-O" in the "extra" leads to the following error because of using snakemake utils for samtools (https://github.com/snakemake/snakemake-wrapper-utils/blob/f9cba17cb380dc7e6729a845ae37f26972c3d1dd/snakemake_wrapper_utils/samtools.py#LL76C23-L76C23):
The fastest and not over-engineered solution to me was to add the gap_opening variable to params, but I feel that it's also not very elegant especially when it comes to modifying the other penalty scores, but they can still be set in the extra.
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,Description by Korbit AI
What change is being made?
Add a parameter for specifying the gap opening penalty in the Minimap2 aligner wrapper and ensure it is correctly passed to the command line.
Why are these changes being made?
Previously, the gap opening penalty option
-Owas not being utilized, which could lead to incorrect alignment results. By adding thegap_openingparameter, users can now specify this penalty, improving the flexibility and accuracy of the alignment process.