-
Notifications
You must be signed in to change notification settings - Fork 195
fix: fix 4118 #4119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: fix 4118 #4119
Conversation
Warning Rate limit exceeded@fgvieira has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe update simplifies the VarScan somatic wrapper input handling by removing explicit separate normal and tumor pileup inputs, instead inferring a single mpileup input when exactly one input file is provided. It removes output directory creation and adjusts the shell command to use the unified input and append logging. The Snakefile input specification was also changed accordingly. Changes
Assessment against linked issues
Suggested reviewers
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bio/varscan/somatic/wrapper.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.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 the `s...
**/*.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 theself
argument of methods.
Do not suggest type annotation of thecls
argument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturn
statement.
bio/varscan/somatic/wrapper.py
`**/wrapper.py`: Do not complain about use of undefined variable called `snakemake`.
**/wrapper.py
: Do not complain about use of undefined variable calledsnakemake
.
bio/varscan/somatic/wrapper.py
🪛 Ruff (0.8.2)
bio/varscan/somatic/wrapper.py
38-38: Undefined name snakemake
(F821)
38-38: Undefined name snakemakeinput
(F821)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: testing
- GitHub Check: docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bio/varscan/somatic/test/Snakefile
(1 hunks)bio/varscan/somatic/wrapper.py
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- bio/varscan/somatic/test/Snakefile
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.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 the `s...
**/*.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 theself
argument of methods.
Do not suggest type annotation of thecls
argument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturn
statement.
bio/varscan/somatic/wrapper.py
`**/wrapper.py`: Do not complain about use of undefined variable called `snakemake`.
**/wrapper.py
: Do not complain about use of undefined variable calledsnakemake
.
bio/varscan/somatic/wrapper.py
🪛 Ruff (0.8.2)
bio/varscan/somatic/wrapper.py
9-9: os
imported but unused
Remove unused import: os
(F401)
15-15: Undefined name snakemake
(F821)
16-16: Undefined name snakemake
(F821)
17-17: Undefined name snakemake
(F821)
20-20: Undefined name op
(F821)
20-20: Undefined name snakemake
(F821)
23-23: Undefined name snakemake
(F821)
🪛 GitHub Actions: Code quality
bio/varscan/somatic/wrapper.py
[error] 1-1: Black formatting check failed. File would be reformatted. Run 'black' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: testing
- GitHub Check: docs
🔇 Additional comments (5)
bio/varscan/somatic/wrapper.py (5)
22-23
: Good simplification of mpileup detection.This elegantly simplifies input handling by automatically detecting a single mpileup input file and setting the appropriate flag. This is a clean approach that aligns with the PR objective to simplify the wrapper.
🧰 Tools
🪛 Ruff (0.8.2)
23-23: Undefined name
snakemake
(F821)
27-27
: Clean input handling simplification.This change simplifies the input handling by directly using the Snakemake input collection instead of explicitly referencing separate pileup files. This works well with the mpileup detection logic added above.
29-30
: Improved parameter organization.Separating Java options and extra parameters onto different lines improves readability and maintainability of the code.
34-34
: Proper logging implementation.Adding the log formatter ensures that both stdout and stderr are properly captured, which is a good practice for debugging and monitoring.
1-36
:❓ Verification inconclusive
Fix Black formatting issues.
The pipeline failure indicates that the file would be reformatted by Black. Please run Black on this file to fix the formatting issues.
🏁 Script executed:
#!/bin/bash # Run Black to check formatting issues black --check bio/varscan/somatic/wrapper.pyLength of output: 142
Install and run Black to fix formatting issues in wrapper.py
The sandbox couldn’t verify formatting (
black
not found). Please install Black (e.g.,pip install black
) and then run:
- black --check bio/varscan/somatic/wrapper.py
- black bio/varscan/somatic/wrapper.py
to address any formatting differences and commit the reformatted file.
🧰 Tools
🪛 Ruff (0.8.2)
9-9:
os
imported but unusedRemove unused import:
os
(F401)
15-15: Undefined name
snakemake
(F821)
16-16: Undefined name
snakemake
(F821)
17-17: Undefined name
snakemake
(F821)
20-20: Undefined name
op
(F821)
20-20: Undefined name
snakemake
(F821)
23-23: Undefined name
snakemake
(F821)
🪛 GitHub Actions: Code quality
[error] 1-1: Black formatting check failed. File would be reformatted. Run 'black' to fix code style issues.
Fixes #4118
QC
snakemake-wrappers
.While the contributions guidelines are more extensive, please particularly ensure that:
test.py
was updated to call any added or updated example rules in aSnakefile
input:
andoutput:
file paths in the rules can be chosen arbitrarilyinput:
oroutput:
)tempfile.gettempdir()
points tometa.yaml
contains a link to the documentation of the respective tool or command underurl:
Summary by CodeRabbit