feat: enable gzip output for bedtools wrappers #3642
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request adds conditional compression functionality to several bedtools wrapper scripts. A new variable Changes
Sequence Diagram(s)sequenceDiagram
participant W as Bedtools Wrapper
participant O as Output Filename
W->>O: Check if filename ends with ".gz"
alt Filename is gzipped
O-->>W: ".gz" detected
W->>W: Set compress = "| bgzip"
else Filename is not gzipped
O-->>W: No ".gz" suffix
W->>W: Set compress = ""
end
W->>S: Execute shell command with compress variable appended
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
bio/bedtools/bamtobed/wrapper.py (1)
12-15: LGTM! Compression logic is correctly implemented.The conditional check for
.gzextension and the corresponding compression command setup is accurate.Optional: Consider using a more concise ternary operator:
-if snakemake.output[0].endswith(".gz"): - compress = "| gzip" -else: - compress = "" +compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""🧰 Tools
🪛 Ruff (0.8.2)
12-15: Use ternary operator
compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""instead ofif-else-blockReplace
if-else-block withcompress = "| gzip" if snakemake.output[0].endswith(".gz") else ""(SIM108)
12-12: Undefined name
snakemake(F821)
bio/bedtools/complement/wrapper.py (1)
10-13: LGTM! Compression logic is correctly implemented.The conditional check for
.gzextension and the corresponding compression command setup is accurate.Optional: Consider using a more concise ternary operator:
-if snakemake.output[0].endswith(".gz"): - compress = "| gzip" -else: - compress = "" +compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""🧰 Tools
🪛 Ruff (0.8.2)
10-13: Use ternary operator
compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""instead ofif-else-blockReplace
if-else-block withcompress = "| gzip" if snakemake.output[0].endswith(".gz") else ""(SIM108)
10-10: Undefined name
snakemake(F821)
bio/bedtools/intersect/wrapper.py (1)
12-16: LGTM! Well-documented compression logic.The compression feature is well-documented and correctly implemented.
Optional: Consider using a more concise ternary operator:
-if snakemake.output[0].endswith(".gz"): - compress = "| gzip" -else: - compress = "" +compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""🧰 Tools
🪛 Ruff (0.8.2)
13-16: Use ternary operator
compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""instead ofif-else-blockReplace
if-else-block withcompress = "| gzip" if snakemake.output[0].endswith(".gz") else ""(SIM108)
13-13: Undefined name
snakemake(F821)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
bio/bedtools/bamtobed/wrapper.py(1 hunks)bio/bedtools/complement/wrapper.py(1 hunks)bio/bedtools/coveragebed/wrapper.py(2 hunks)bio/bedtools/genomecov/wrapper.py(1 hunks)bio/bedtools/intersect/test/Snakefile(1 hunks)bio/bedtools/intersect/wrapper.py(1 hunks)bio/bedtools/merge/wrapper.py(2 hunks)test_wrappers.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
bio/bedtools/coveragebed/wrapper.pybio/bedtools/merge/wrapper.pybio/bedtools/intersect/wrapper.pybio/bedtools/complement/wrapper.pybio/bedtools/bamtobed/wrapper.pybio/bedtools/genomecov/wrapper.pytest_wrappers.py
`**/wrapper.py`: Do not complain about use of undefined vari...
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
bio/bedtools/coveragebed/wrapper.pybio/bedtools/merge/wrapper.pybio/bedtools/intersect/wrapper.pybio/bedtools/complement/wrapper.pybio/bedtools/bamtobed/wrapper.pybio/bedtools/genomecov/wrapper.py
🪛 Ruff (0.8.2)
bio/bedtools/coveragebed/wrapper.py
20-23: Use ternary operator compress = "| gzip" if output_file.endswith(".gz") else "" instead of if-else-block
Replace if-else-block with compress = "| gzip" if output_file.endswith(".gz") else ""
(SIM108)
bio/bedtools/merge/wrapper.py
13-16: Use ternary operator compress = "| gzip" if snakemake.output[0].endswith(".gz") else "" instead of if-else-block
Replace if-else-block with compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""
(SIM108)
13-13: Undefined name snakemake
(F821)
bio/bedtools/intersect/wrapper.py
13-16: Use ternary operator compress = "| gzip" if snakemake.output[0].endswith(".gz") else "" instead of if-else-block
Replace if-else-block with compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""
(SIM108)
13-13: Undefined name snakemake
(F821)
bio/bedtools/complement/wrapper.py
10-13: Use ternary operator compress = "| gzip" if snakemake.output[0].endswith(".gz") else "" instead of if-else-block
Replace if-else-block with compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""
(SIM108)
10-10: Undefined name snakemake
(F821)
16-16: Undefined name snakemake
(F821)
bio/bedtools/bamtobed/wrapper.py
12-15: Use ternary operator compress = "| gzip" if snakemake.output[0].endswith(".gz") else "" instead of if-else-block
Replace if-else-block with compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""
(SIM108)
12-12: Undefined name snakemake
(F821)
bio/bedtools/genomecov/wrapper.py
22-25: Use ternary operator compress = "| gzip" if snakemake.output[0].endswith(".gz") else "" instead of if-else-block
Replace if-else-block with compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""
(SIM108)
22-22: Undefined name snakemake
(F821)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (13)
bio/bedtools/bamtobed/wrapper.py (1)
23-23: LGTM! Shell command is correctly modified.The compression command is properly placed in the shell command before the output redirection.
bio/bedtools/complement/wrapper.py (1)
23-23: LGTM! Shell command is correctly modified.The compression command is properly placed in the shell command before the output redirection.
bio/bedtools/intersect/wrapper.py (1)
23-23: LGTM! Shell command is correctly modified.The compression command is properly placed in the shell command before the output redirection.
bio/bedtools/intersect/test/Snakefile (1)
16-28: Consider using a unique log file for the gzipped output rule.The new rule is well-structured and correctly implements gzipped output. However, both rules are using the same log file path which could lead to log overwrites if both rules are executed in the same workflow.
Consider using a unique log file path for the gzipped output rule:
log: - "logs/intersect/A_B.log" + "logs/intersect/A_B.gz.log"✅ Verification successful
Unique log file needed for gzipped output rule
The current implementation in rule bedtools_merge_zip is using the log file "logs/intersect/A_B.log", which is shared with another rule. This introduces the possibility of log overwrites when both rules run in the same workflow. Changing the log file to a unique name (e.g., "logs/intersect/A_B.gz.log") for the gzipped output rule—as suggested—is appropriate.
- In bio/bedtools/intersect/test/Snakefile (lines 16–28), change the log file path to "logs/intersect/A_B.gz.log".
bio/bedtools/coveragebed/wrapper.py (2)
20-26: LGTM! Compression detection logic is correctly implemented.The implementation correctly detects if the output file should be compressed based on the
.gzextension.🧰 Tools
🪛 Ruff (0.8.2)
20-23: Use ternary operator
compress = "| gzip" if output_file.endswith(".gz") else ""instead ofif-else-blockReplace
if-else-block withcompress = "| gzip" if output_file.endswith(".gz") else ""(SIM108)
35-35: LGTM! Compression pipe is correctly placed.The compression pipe is correctly placed before the output redirection, ensuring proper handling of gzipped output.
bio/bedtools/genomecov/wrapper.py (2)
22-26: LGTM! Compression detection logic is correctly implemented.The implementation correctly detects if the output file should be compressed based on the
.gzextension.🧰 Tools
🪛 Ruff (0.8.2)
22-25: Use ternary operator
compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""instead ofif-else-blockReplace
if-else-block withcompress = "| gzip" if snakemake.output[0].endswith(".gz") else ""(SIM108)
22-22: Undefined name
snakemake(F821)
32-32: LGTM! Compression pipe is correctly placed.The compression pipe is correctly placed before the output redirection, ensuring proper handling of gzipped output.
bio/bedtools/merge/wrapper.py (3)
13-18: LGTM! Compression detection logic is correctly implemented.The implementation correctly detects if the output file should be compressed based on the
.gzextension.🧰 Tools
🪛 Ruff (0.8.2)
13-16: Use ternary operator
compress = "| gzip" if snakemake.output[0].endswith(".gz") else ""instead ofif-else-blockReplace
if-else-block withcompress = "| gzip" if snakemake.output[0].endswith(".gz") else ""(SIM108)
13-13: Undefined name
snakemake(F821)
30-32: LGTM! Compression pipe is correctly placed for multiple input files.The compression pipe is correctly placed before the output redirection when handling multiple input files.
40-41: LGTM! Compression pipe is correctly placed for single input file.The compression pipe is correctly placed before the output redirection when handling a single input file.
test_wrappers.py (2)
1957-1957: LGTM! Appropriate flag addition.The addition of the
-Fflag ensures consistent behavior with force overwrite.
1984-1989: LGTM! Good test coverage for gzip output functionality.The new test function properly verifies the gzip output capability for bedtools intersect.
Let's verify that the test covers all key aspects of the gzip functionality:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the test covers all necessary aspects of gzip output # Check if the corresponding wrapper handles gzip detection and compression ast-grep --pattern $'if output.endswith(".gz"): $$$ "| gzip" $$$'Length of output: 72
I'll run an additional script to inspect the new test function in "test_wrappers.py" and check for any gzip handling logic in the "bio/bedtools/intersect" wrapper. This should help us verify that the test indeed exercises the gzip-related functionality.
🏁 Script executed:
#!/bin/bash echo "Examining the new test function in test_wrappers.py (lines 1980-1995):" sed -n '1980,1995p' test_wrappers.py echo "" echo "Searching for gzip-related logic in the bedtools intersect wrapper (bio/bedtools/intersect):" rg "\.gz" bio/bedtools/intersect -nLength of output: 986
LGTM: The new test function properly exercises the gzip-specific functionality.
• The new test calls the wrapper with an output filename ending in ".bed.gz", and the wrapper confirms gzip handling by checking for ".gz" in the output.
• The additional "-F" flag in other tests remains consistent with expected behavior.
gtuckerkellogg
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
bio/bedtools/coveragebed/wrapper.py (1)
25-33: Fix the compression pipe placement in the shell command.The compression pipe is incorrectly placed before the output redirection, which will cause the compression to be applied to the command itself rather than its output.
Apply this diff to fix the shell command:
shell( "coverageBed" " -a {input_a}" " -b {input_b}" " {extra_params}" - " {compress}" " > {output_file}" + " {compress}" " {log}" )
🧹 Nitpick comments (2)
bio/bedtools/bamtobed/test/Snakefile (1)
14-24: Fix log file path to avoid conflicts.The new rule looks good, but it shares the same log file path with the
bamtobedrule. This could cause issues when running both rules in parallel.Apply this diff to use a unique log file path:
- "logs/bamtobed/{sample}.log", + "logs/bamtobed/{sample}.gz.log",test_wrappers.py (1)
1987-1995: Clean up formatting and remove stray characterThe test case has inconsistent whitespace and a stray 'r' character.
- [ - "snakemake", - "--cores", - "1", - "A_B.intersected.bed", - "A_B.intersected.bed.gz", - "--use-conda", - "-F" - r], + [ + "snakemake", + "--cores", + "1", + "A_B.intersected.bed", + "A_B.intersected.bed.gz", + "--use-conda", + "-F" + ],🧰 Tools
🪛 Ruff (0.8.2)
1995-1995: SyntaxError: Expected ',', found name
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bio/bedtools/complement/test/expected/bed-complement/a.complement.bed.gzis excluded by!**/*.gzbio/bedtools/intersect/test/expected/A_B.intersected.bed.gzis excluded by!**/*.gz
📒 Files selected for processing (13)
bio/bedtools/bamtobed/test/Snakefile(2 hunks)bio/bedtools/bamtobed/wrapper.py(1 hunks)bio/bedtools/complement/test/Snakefile(1 hunks)bio/bedtools/complement/wrapper.py(1 hunks)bio/bedtools/coveragebed/wrapper.py(2 hunks)bio/bedtools/genomecov/wrapper.py(1 hunks)bio/bedtools/intersect/test/Snakefile(2 hunks)bio/bedtools/intersect/wrapper.py(1 hunks)bio/bedtools/merge/wrapper.py(2 hunks)bio/bedtools/slop/wrapper.py(1 hunks)bio/bedtools/sort/wrapper.py(2 hunks)bio/bedtools/split/wrapper.py(2 hunks)test_wrappers.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- bio/bedtools/intersect/wrapper.py
- bio/bedtools/bamtobed/wrapper.py
- bio/bedtools/intersect/test/Snakefile
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
bio/bedtools/split/wrapper.pybio/bedtools/slop/wrapper.pybio/bedtools/sort/wrapper.pytest_wrappers.pybio/bedtools/complement/wrapper.pybio/bedtools/coveragebed/wrapper.pybio/bedtools/genomecov/wrapper.pybio/bedtools/merge/wrapper.py
`**/wrapper.py`: Do not complain about use of undefined vari...
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
bio/bedtools/split/wrapper.pybio/bedtools/slop/wrapper.pybio/bedtools/sort/wrapper.pybio/bedtools/complement/wrapper.pybio/bedtools/coveragebed/wrapper.pybio/bedtools/genomecov/wrapper.pybio/bedtools/merge/wrapper.py
🪛 Ruff (0.8.2)
bio/bedtools/split/wrapper.py
13-13: Undefined name snakemake
(F821)
bio/bedtools/slop/wrapper.py
13-13: Undefined name snakemake
(F821)
bio/bedtools/sort/wrapper.py
14-14: Undefined name snakemake
(F821)
test_wrappers.py
1995-1995: SyntaxError: Expected ',', found name
bio/bedtools/complement/wrapper.py
8-8: Undefined name snakemake
(F821)
10-10: Undefined name snakemake
(F821)
12-12: Undefined name snakemake
(F821)
bio/bedtools/coveragebed/wrapper.py
20-20: Undefined name snakemake
(F821)
bio/bedtools/genomecov/wrapper.py
22-22: Undefined name snakemake
(F821)
bio/bedtools/merge/wrapper.py
13-13: Undefined name snakemake
(F821)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (13)
bio/bedtools/slop/wrapper.py (2)
13-13: LGTM!The compression logic is simple and effective.
🧰 Tools
🪛 Ruff (0.8.2)
13-13: Undefined name
snakemake(F821)
20-20: LGTM!The shell command is properly modified to include compression before output redirection.
bio/bedtools/complement/wrapper.py (2)
10-10: LGTM!The compression logic is simple and effective.
🧰 Tools
🪛 Ruff (0.8.2)
10-10: Undefined name
snakemake(F821)
19-19: LGTM!The shell command is properly modified to include compression before output redirection.
bio/bedtools/sort/wrapper.py (2)
14-14: LGTM!The compression logic is simple and effective.
🧰 Tools
🪛 Ruff (0.8.2)
14-14: Undefined name
snakemake(F821)
25-25: LGTM!The shell command is properly modified to include compression before output redirection.
bio/bedtools/split/wrapper.py (2)
13-13: LGTM! Compression check is correctly implemented.The compression check is consistent with the implementation across other bedtools wrappers.
🧰 Tools
🪛 Ruff (0.8.2)
13-13: Undefined name
snakemake(F821)
28-28: LGTM! Shell command correctly handles compression.The shell command appropriately pipes the output through gzip when the output file has a
.gzextension.bio/bedtools/complement/test/Snakefile (1)
15-28: LGTM! New rule for gzipped output is correctly implemented.The new rule
bedtools_complement_bed_gzis properly configured with appropriate input files, parameters, output path with.gzextension, and corresponding log path.bio/bedtools/merge/wrapper.py (3)
13-13: LGTM! Clean implementation of gzip output detection.The ternary operator elegantly handles the compression logic based on the output filename extension.
🧰 Tools
🪛 Ruff (0.8.2)
13-13: Undefined name
snakemake(F821)
26-28: LGTM! Proper placement of compression in the pipeline.The compression command is correctly positioned before the output redirection, ensuring that the entire output is compressed when needed.
36-37: LGTM! Consistent compression handling for single input.The compression command is correctly positioned and maintains consistency with the multiple input file case.
test_wrappers.py (1)
1912-1913: LGTM! The changes align with the PR objectivesThe modifications to test cases for bedtools wrappers properly test both uncompressed and compressed outputs. The addition of result verification using
compare_results_with_expectedis a good practice to ensure output correctness.Also applies to: 1939-1949, 1987-1999
|
I think it is a very good idea to add |
|
You are correct. I'll fix it. |
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 (11)
.#test_wrappers.py(1 hunks)bio/bedtools/.vscode/settings.json(1 hunks)bio/bedtools/bamtobed/wrapper.py(1 hunks)bio/bedtools/complement/wrapper.py(1 hunks)bio/bedtools/coveragebed/wrapper.py(3 hunks)bio/bedtools/genomecov/wrapper.py(1 hunks)bio/bedtools/intersect/wrapper.py(1 hunks)bio/bedtools/merge/wrapper.py(2 hunks)bio/bedtools/slop/wrapper.py(1 hunks)bio/bedtools/sort/wrapper.py(2 hunks)bio/bedtools/split/wrapper.py(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- bio/bedtools/.vscode/settings.json
🚧 Files skipped from review as they are similar to previous changes (3)
- bio/bedtools/coveragebed/wrapper.py
- bio/bedtools/bamtobed/wrapper.py
- bio/bedtools/intersect/wrapper.py
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
bio/bedtools/complement/wrapper.pybio/bedtools/genomecov/wrapper.pybio/bedtools/merge/wrapper.pybio/bedtools/slop/wrapper.pybio/bedtools/sort/wrapper.pybio/bedtools/split/wrapper.py
`**/wrapper.py`: Do not complain about use of undefined vari...
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
bio/bedtools/complement/wrapper.pybio/bedtools/genomecov/wrapper.pybio/bedtools/merge/wrapper.pybio/bedtools/slop/wrapper.pybio/bedtools/sort/wrapper.pybio/bedtools/split/wrapper.py
🪛 Ruff (0.8.2)
.#test_wrappers.py
1-1: No such file or directory (os error 2)
(E902)
bio/bedtools/complement/wrapper.py
8-8: Undefined name snakemake
(F821)
10-10: Undefined name snakemake
(F821)
12-12: Undefined name snakemake
(F821)
bio/bedtools/genomecov/wrapper.py
22-22: Undefined name snakemake
(F821)
bio/bedtools/merge/wrapper.py
13-13: Undefined name snakemake
(F821)
bio/bedtools/slop/wrapper.py
13-13: Undefined name snakemake
(F821)
bio/bedtools/sort/wrapper.py
14-14: Undefined name snakemake
(F821)
bio/bedtools/split/wrapper.py
13-13: Undefined name snakemake
(F821)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (9)
bio/bedtools/slop/wrapper.py (1)
13-13: LGTM! The bgzip compression implementation looks good.The implementation correctly:
- Checks the output file extension to determine if compression is needed
- Places the compression command before the output redirection
Also applies to: 20-20
🧰 Tools
🪛 Ruff (0.8.2)
13-13: Undefined name
snakemake(F821)
bio/bedtools/complement/wrapper.py (1)
6-6: LGTM! The bgzip compression implementation looks good.The implementation correctly:
- Checks the output file extension to determine if compression is needed
- Places the compression command before the output redirection
Also applies to: 10-10, 19-19
bio/bedtools/sort/wrapper.py (1)
14-14: LGTM! The bgzip compression implementation looks good.The implementation correctly:
- Checks the output file extension to determine if compression is needed
- Places the compression command before the output redirection
Also applies to: 25-25
🧰 Tools
🪛 Ruff (0.8.2)
14-14: Undefined name
snakemake(F821)
bio/bedtools/split/wrapper.py (1)
13-13: LGTM! The bgzip compression implementation looks good.The implementation correctly:
- Checks the output file extension to determine if compression is needed
- Places the compression command before the output redirection
Also applies to: 28-28
🧰 Tools
🪛 Ruff (0.8.2)
13-13: Undefined name
snakemake(F821)
bio/bedtools/genomecov/wrapper.py (2)
22-23: LGTM!The implementation correctly uses
bgzipfor compression, which allows for indexing with tools liketabix.🧰 Tools
🪛 Ruff (0.8.2)
22-22: Undefined name
snakemake(F821)
24-31: LGTM!The shell command correctly places the compression pipe before output redirection, ensuring proper handling of standard output.
bio/bedtools/merge/wrapper.py (3)
12-14: LGTM!The implementation correctly uses
bgzipfor compression, which allows for indexing with tools liketabix.🧰 Tools
🪛 Ruff (0.8.2)
13-13: Undefined name
snakemake(F821)
26-28: LGTM!The shell command correctly places the compression pipe before output redirection, ensuring proper handling of standard output.
36-37: LGTM!The shell command correctly places the compression pipe before output redirection, ensuring proper handling of standard output.
…t' into bedtools-gzip-output
Co-authored-by: Filipe G. Vieira <[email protected]>
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)
bio/bedtools/complement/wrapper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.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.
bio/bedtools/complement/wrapper.py
`**/wrapper.py`: Do not complain about use of undefined vari...
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
bio/bedtools/complement/wrapper.py
🪛 Ruff (0.8.2)
bio/bedtools/complement/wrapper.py
10-10: Undefined name snakemake
(F821)
12-12: Undefined name snakemake
(F821)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (1)
bio/bedtools/complement/wrapper.py (1)
19-19: LGTM!The compression command is correctly positioned in the pipeline, before the output redirection.
|
I guess file bio/bedtools/.vscode/settings.json is also not needed. 😄 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test_wrappers.py (1)
1984-2000: Fix indentation inconsistencies in the test_bedtools_intersect function.The code has inconsistent indentation with extra spaces. Consider fixing:
- [ - "snakemake", - "--cores", - "1", - "A_B.intersected.bed", - "A_B.intersected.bed.gz", - "--use-conda", - "-F" - ], - compare_results_with_expected={ + [ + "snakemake", + "--cores", + "1", + "A_B.intersected.bed", + "A_B.intersected.bed.gz", + "--use-conda", + "-F" + ], + compare_results_with_expected={
📜 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`: Do not try to improve formatting. Do not suggest ...
**/*.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.
test_wrappers.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (2)
test_wrappers.py (2)
1909-1913: LGTM! Added gzip output test for bamtobed.The change correctly adds testing for gzipped output file format.
1931-1949: LGTM! Added gzip output test with result validation for complement.The changes properly add:
- Gzipped output file testing
- Result comparison with expected files
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test_wrappers.py (1)
1987-1995: Improve code formatting for consistencyThe list formatting is inconsistent with the rest of the file. Consider removing extra whitespace after brackets and quotes.
- [ - "snakemake", - "--cores", - "1", - "A_B.intersected.bed", - "A_B.intersected.bed.gz", - "--use-conda", - "-F" - ], + [ + "snakemake", + "--cores", + "1", + "A_B.intersected.bed", + "A_B.intersected.bed.gz", + "--use-conda", + "-F" + ],
📜 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`: Do not try to improve formatting. Do not suggest ...
**/*.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.
test_wrappers.py
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (3)
test_wrappers.py (3)
1909-1913: LGTM!The changes to add gzipped output testing for bedtools bamtobed look good.
1931-1949: LGTM!The changes to add gzipped output testing and expected result comparison for bedtools complement look good.
1996-1999: LGTM!The addition of expected result comparison for bedtools intersect is good.
Co-authored-by: Filipe G. Vieira <[email protected]>
|
@gtuckerkellogg can you also update the |
done |
|
@fgvieira I'm not familiar with the full set of testing actions triggered on push, and don't understand the test failure in the current commit. |
|
The test is failing because it says the files |
Enable gzip output for all current bedtools wrappers.
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
Tests
Chores