feat: added wrapper for Paraphase #3071
Conversation
…d BAMs as input and phases haplotypes for genes of the same family, determines copy numbers and makes phased variant calls.
|
Hello snakemake-wrapper maintainers. Thank you for the great job you are doing! This wrapper is for the Paraphase software https://github.com/PacificBiosciences/paraphase. As test set I picked the smallest possible slice of a chromosome which contained two gene copies close to the chromosome start. After discussing with Paraphase developers this seemed the most efficient way to make a small data slice, without including a whole reference genome. The software only works for specific versions of the human genome. This is the first wrapper I've created, so I'm happy to accept and implement suggestions for improvements. |
|
thank you so much for the contribution! |
Co-authored-by: Johannes Köster <[email protected]>
Co-authored-by: Johannes Köster <[email protected]>
Co-authored-by: Johannes Köster <[email protected]>
MagdalenaZZ
changed the title
|
Thank you @johanneskoester for the review and suggestions, all incorporated. |
|
I think I've made all the changes requested? |
|
Thanks for your contribution. Some questions:
|
Co-authored-by: Filipe G. Vieira <[email protected]>
Co-authored-by: Filipe G. Vieira <[email protected]>
Co-authored-by: Filipe G. Vieira <[email protected]>
|
And remember to add the tests to |
Co-authored-by: Filipe G. Vieira <[email protected]>
Removed exception
|
I now pushed a change that will resolve the Code quality / formatting (pull_request) linting error. |
![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
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bio/paraphase/wrapper.py (1 hunks)
Additional context used
Path-based instructions (1)
bio/paraphase/wrapper.py (1)
Pattern
**/*.py: Do not suggest to add trailing commas for improving formatting.
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 explicitly return anything.
Ruff
bio/paraphase/wrapper.py
1-1:
osimported but unusedRemove unused import:
os(F401)
6-6:
shutilimported but unusedRemove unused import:
shutil(F401)
8-8: Undefined name
snakemake(F821)
9-9: Undefined name
snakemake(F821)
17-17: Undefined name
snakemake(F821)
18-18: Undefined name
snakemake(F821)
20-20: Undefined name
snakemake(F821)
28-28: Trailing comma missing
Add trailing comma
(COM812)
34-34: Undefined name
snakemake(F821)
35-35: Undefined name
snakemake(F821)
38-38: Unnecessary open mode parameters
Remove open mode parameters
(UP015)
64-64: Undefined name
snakemake(F821)
65-65: Undefined name
snakemake(F821)
65-65: Trailing comma missing
Add trailing comma
(COM812)
68-68: Undefined name
snakemake(F821)
68-68: Trailing comma missing
Add trailing comma
(COM812)
77-77: Undefined name
snakemake(F821)
78-78: Undefined name
snakemake(F821)
79-79: Trailing comma missing
Add trailing comma
(COM812)
83-83: Trailing comma missing
Add trailing comma
(COM812)
85-85: Undefined name
snakemake(F821)
87-87: Do not catch blind exception:
Exception(BLE001)
Additional comments not posted (4)
bio/paraphase/wrapper.py (4)
1-6: Remove unused imports.The imports
osandshutilare not used in the code and should be removed to clean up the file.Tools
Ruff
1-1:
osimported but unusedRemove unused import:
os(F401)
6-6:
shutilimported but unusedRemove unused import:
shutil(F401)
11-24: LGTM! Use ofTemporaryDirectoryand shell execution is appropriate.The use of
TemporaryDirectoryensures that temporary files are managed correctly, and the shell command is constructed in a clear manner.Tools
Ruff
17-17: Undefined name
snakemake(F821)
18-18: Undefined name
snakemake(F821)
20-20: Undefined name
snakemake(F821)
51-80: Comprehensive VCF and BAM file processing.The logic for compressing, indexing, and processing VCF files, as well as copying BAM files, is well-structured. Ensure robust error handling for shell commands.
Verification successful
Robust Error Handling for Shell Commands in
bio/paraphase/wrapper.py.The shell commands in
bio/paraphase/wrapper.pyare wrapped in a try-except block, providing necessary error handling by catching exceptions and printing error messages. This ensures robustness in the script's execution. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that shell commands handle errors correctly. # Test: Search for error handling in shell command execution. rg --type python 'shell\(' -A 3Length of output: 64
Script:
#!/bin/bash # Description: Verify that shell commands handle errors correctly. # Corrected Test: Search for error handling in shell command execution in Python files. rg 'shell\(' -A 3 --glob '*.py'Length of output: 206621
Tools
Ruff
64-64: Undefined name
snakemake(F821)
65-65: Undefined name
snakemake(F821)
65-65: Trailing comma missing
Add trailing comma
(COM812)
68-68: Undefined name
snakemake(F821)
68-68: Trailing comma missing
Add trailing comma
(COM812)
77-77: Undefined name
snakemake(F821)
78-78: Undefined name
snakemake(F821)
79-79: Trailing comma missing
Add trailing comma
(COM812)
8-8: Ensuresnakemakeis defined.The
snakemakeobject is used but not defined in this file. Ensure it is passed to the script or imported correctly.Verification successful
snakemakeis correctly imported inbio/paraphase/wrapper.py.The
snakemakeobject is imported fromsnakemake.shell, which is a common pattern for Snakemake wrappers. This ensures thatsnakemakeis defined and available for use in the script.
- The import statement
from snakemake.shell import shellis present inbio/paraphase/wrapper.py.This confirms that
snakemakeis properly set up for use in this file. No further action is needed regarding the import or definition ofsnakemake.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `snakemake` is defined or imported in the context where this script is used. # Test: Search for the definition or import of `snakemake` in related files. rg --type python 'snakemake'Length of output: 59
Script:
#!/bin/bash # Description: Correctly verify if `snakemake` is defined or imported in the context where this script is used. # Test: Search for the definition or import of `snakemake` in Python files. rg 'snakemake' --glob '*.py'Length of output: 339444
Script:
#!/bin/bash # Description: Check for any import or mention of `snakemake` in `bio/paraphase/wrapper.py`. # Search for `snakemake` in `bio/paraphase/wrapper.py`. rg 'snakemake' bio/paraphase/wrapper.pyLength of output: 895
Tools
Ruff
8-8: Undefined name
snakemake(F821)
|
Can someone please address the change requested? |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (3)
bio/paraphase/meta.yaml (1)
13-14: Remove trailing spaces.Trailing spaces are present on these lines. Remove them to adhere to YAML formatting standards.
Fix the trailing spaces as follows:
- bai: bai index file for the realigned reads - merged_vcf: VCF file containing all newly discovered variants, phased by gene copy and haplotype + bai: bai index file for the realigned reads + merged_vcf: VCF file containing all newly discovered variants, phased by gene copy and haplotypebio/paraphase/wrapper.py (2)
1-1: Remove unused imports.The imports
osandshutilare not used in the code and should be removed to clean up the file.Remove the unused imports as follows:
- import os - import shutil
46-46: Remove unnecessaryclosecall.The
fai_file.close()call is unnecessary because thewithstatement automatically handles file closing.Remove the unnecessary close call as follows:
- output.close()
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- bio/paraphase/meta.yaml (1 hunks)
- bio/paraphase/wrapper.py (1 hunks)
Additional context used
Path-based instructions (1)
bio/paraphase/wrapper.py (1)
Pattern
**/*.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.
Learnings (1)
bio/paraphase/wrapper.py (1)
Learnt from: fgvieira PR: snakemake/snakemake-wrappers#3071 File: bio/paraphase/wrapper.py:84-86 Timestamp: 2024-08-20T19:23:34.259Z Learning: In Snakemake wrappers, a try/except block is unnecessary because Snakemake will handle exceptions if the wrapper fails.
yamllint
bio/paraphase/meta.yaml
[error] 2-2: duplication of key "name" in mapping
(key-duplicates)
Ruff
bio/paraphase/wrapper.py
6-6: Undefined name
snakemake(F821)
7-7: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
16-16: Undefined name
snakemake(F821)
18-18: Undefined name
snakemake(F821)
32-32: Undefined name
snakemake(F821)
33-33: Undefined name
snakemake(F821)
61-61: Undefined name
snakemake(F821)
62-62: Undefined name
snakemake(F821)
65-65: Undefined name
snakemake(F821)
74-74: Undefined name
snakemake(F821)
75-75: Undefined name
snakemake(F821)
82-82: Undefined name
snakemake(F821)
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
bio/paraphase/wrapper.py (1)
48-82: Consider replacing print statements with logging.Replacing print statements with logging would be more consistent with Snakemake's logging practices.
Use this diff to replace the print statements:
- print(f"Compressed and indexed: {vcf}.gz") + log.info(f"Compressed and indexed: {vcf}.gz") - print(f"Merged, reheadered, and sorted VCF file created: {snakemake.output.merged_vcf}") + log.info(f"Merged, reheadered, and sorted VCF file created: {snakemake.output.merged_vcf}") - print("No output VCF or BAM files were produced by paraphase, I hope this is what you were expecting, human?") + log.warning("No output VCF or BAM files were produced by paraphase, I hope this is what you were expecting, human?")Tools
Ruff
52-52: SyntaxError: unindent does not match any outer indentation level
53-53: SyntaxError: Unexpected indentation
58-58: SyntaxError: Unexpected indentation
78-78: SyntaxError: Expected a statement
78-78: SyntaxError: Expected a statement
78-79: SyntaxError: Expected a statement
79-79: SyntaxError: Unexpected indentation
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bio/paraphase/wrapper.py (1 hunks)
Additional context used
Path-based instructions (1)
bio/paraphase/wrapper.py (1)
Pattern
**/*.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.
Learnings (1)
bio/paraphase/wrapper.py (2)
Learnt from: MagdalenaZZ PR: snakemake/snakemake-wrappers#3071 File: bio/paraphase/wrapper.py:0-0 Timestamp: 2024-08-21T08:27:20.017Z Learning: In Snakemake wrappers, a try/except block is unnecessary because Snakemake will handle exceptions if the wrapper fails.Learnt from: fgvieira PR: snakemake/snakemake-wrappers#3071 File: bio/paraphase/wrapper.py:84-86 Timestamp: 2024-08-20T19:23:34.259Z Learning: In Snakemake wrappers, a try/except block is unnecessary because Snakemake will handle exceptions if the wrapper fails.
Ruff
bio/paraphase/wrapper.py
42-42: SyntaxError: unindent does not match any outer indentation level
45-45: SyntaxError: Unexpected indentation
52-52: SyntaxError: unindent does not match any outer indentation level
53-53: SyntaxError: Unexpected indentation
58-58: SyntaxError: Unexpected indentation
78-78: SyntaxError: Expected a statement
78-78: SyntaxError: Expected a statement
78-79: SyntaxError: Expected a statement
79-79: SyntaxError: Unexpected indentation
89-89: SyntaxError: unindent does not match any outer indentation level
Additional comments not posted (3)
bio/paraphase/wrapper.py (3)
1-8: Imports and initial setup look good.The imports and initial setup are appropriate for the functionality provided by the wrapper.
10-22: TemporaryDirectory usage andparaphaseexecution look good.The use of
TemporaryDirectoryfor managing temporary files and the construction of theparaphasecommand using Snakemake inputs and parameters are appropriate.
89-162: Remove duplicate code block.The code block from line 89 onwards is a duplicate of the previous code and should be removed to avoid redundancy.
Use this diff to remove the duplicate code block:
- with TemporaryDirectory() as tmpdirname: - ...Likely invalid or redundant comment.
Tools
Ruff
89-89: SyntaxError: unindent does not match any outer indentation level
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
In the file I sorted some formatting issues, hopefully the checks will work now. |
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- bio/paraphase/meta.yaml (1 hunks)
- bio/paraphase/wrapper.py (1 hunks)
Files skipped from review due to trivial changes (1)
- bio/paraphase/meta.yaml
Additional context used
Path-based instructions (1)
bio/paraphase/wrapper.py (2)
Pattern
**/*.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.
Pattern
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
Learnings (1)
bio/paraphase/wrapper.py (2)
Learnt from: MagdalenaZZ PR: snakemake/snakemake-wrappers#3071 File: bio/paraphase/wrapper.py:0-0 Timestamp: 2024-08-21T08:27:20.017Z Learning: In Snakemake wrappers, a try/except block is unnecessary because Snakemake will handle exceptions if the wrapper fails.Learnt from: fgvieira PR: snakemake/snakemake-wrappers#3071 File: bio/paraphase/wrapper.py:84-86 Timestamp: 2024-08-20T19:23:34.259Z Learning: In Snakemake wrappers, a try/except block is unnecessary because Snakemake will handle exceptions if the wrapper fails.
Ruff
bio/paraphase/wrapper.py
2-2:
sysimported but unusedRemove unused import:
sys(F401)
6-6: Undefined name
snakemake(F821)
7-7: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
16-16: Undefined name
snakemake(F821)
18-18: Undefined name
snakemake(F821)
32-32: Undefined name
snakemake(F821)
33-33: Undefined name
snakemake(F821)
58-58: Undefined name
snakemake(F821)
59-59: Undefined name
snakemake(F821)
61-61: Undefined name
snakemake(F821)
69-69: Undefined name
snakemake(F821)
70-70: Undefined name
snakemake(F821)
75-75: Undefined name
snakemake(F821)
83-83: Undefined name
snakemake(F821)
84-84: Undefined name
snakemake(F821)
86-86: Undefined name
snakemake(F821)
100-100: Undefined name
snakemake(F821)
101-101: Undefined name
snakemake(F821)
127-127: Undefined name
snakemake(F821)
128-128: Undefined name
snakemake(F821)
130-130: Undefined name
snakemake(F821)
138-138: Undefined name
snakemake(F821)
139-139: Undefined name
snakemake(F821)
144-144: Undefined name
snakemake(F821)
Additional comments not posted (4)
bio/paraphase/wrapper.py (4)
10-22: LGTM!The code correctly uses a temporary directory and runs the
paraphasecommand with appropriate parameters and logging.The code changes are approved.
Tools
Ruff
15-15: Undefined name
snakemake(F821)
16-16: Undefined name
snakemake(F821)
18-18: Undefined name
snakemake(F821)
23-27: LGTM!The
touchcommand is correctly used to create the output file.The code changes are approved.
29-44: LGTM!The code correctly reads the
.faiindex file and writes formatted header lines to the output VCF header file.The code changes are approved.
Tools
Ruff
32-32: Undefined name
snakemake(F821)
33-33: Undefined name
snakemake(F821)
45-75: LGTM!The code correctly processes VCF files and copies BAM and BAI files. The print statements provide useful information about the processing steps.
The code changes are approved.
Tools
Ruff
58-58: Undefined name
snakemake(F821)
59-59: Undefined name
snakemake(F821)
61-61: Undefined name
snakemake(F821)
69-69: Undefined name
snakemake(F821)
70-70: Undefined name
snakemake(F821)
75-75: Undefined name
snakemake(F821)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bio/paraphase/wrapper.py (1 hunks)
Additional context used
Path-based instructions (1)
bio/paraphase/wrapper.py (2)
Pattern
**/*.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.
Pattern
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
Ruff
bio/paraphase/wrapper.py
5-5: Undefined name
snakemake(F821)
6-6: Undefined name
snakemake(F821)
14-14: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
17-17: Undefined name
snakemake(F821)
31-31: Undefined name
snakemake(F821)
32-32: Undefined name
snakemake(F821)
57-57: Undefined name
snakemake(F821)
58-58: Undefined name
snakemake(F821)
60-60: Undefined name
snakemake(F821)
68-68: Undefined name
snakemake(F821)
69-69: Undefined name
snakemake(F821)
74-74: Undefined name
snakemake(F821)
82-82: Undefined name
snakemake(F821)
83-83: Undefined name
snakemake(F821)
85-85: Undefined name
snakemake(F821)
99-99: Undefined name
snakemake(F821)
100-100: Undefined name
snakemake(F821)
126-126: Undefined name
snakemake(F821)
127-127: Undefined name
snakemake(F821)
129-129: Undefined name
snakemake(F821)
137-137: Undefined name
snakemake(F821)
138-138: Undefined name
snakemake(F821)
143-143: Undefined name
snakemake(F821)
Additional comments not posted (6)
bio/paraphase/wrapper.py (6)
1-7: LGTM!The imports and initial setup are appropriate for the functionality described.
The code changes are approved.
Tools
Ruff
5-5: Undefined name
snakemake(F821)
6-6: Undefined name
snakemake(F821)
9-21: LGTM!The temporary directory creation and
paraphaseexecution are correctly implemented.The code changes are approved.
Tools
Ruff
14-14: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
17-17: Undefined name
snakemake(F821)
22-26: LGTM!The use of the
touchcommand is appropriate for ensuring the output file exists.The code changes are approved.
28-43: LGTM!The VCF header creation is correctly implemented.
The code changes are approved.
Tools
Ruff
31-31: Undefined name
snakemake(F821)
32-32: Undefined name
snakemake(F821)
44-75: LGTM!The VCF file handling and BAM/BAI file copying are correctly implemented.
The code changes are approved.
Tools
Ruff
57-57: Undefined name
snakemake(F821)
58-58: Undefined name
snakemake(F821)
60-60: Undefined name
snakemake(F821)
68-68: Undefined name
snakemake(F821)
69-69: Undefined name
snakemake(F821)
74-74: Undefined name
snakemake(F821)
113-143: Remove duplicate code.This segment is a duplicate of the previous code for handling VCF files and copying BAM/BAI files. Consider removing it to avoid redundancy.
Apply this diff to remove the duplicate code:
-### Concatenating, reheadering, and sorting the zipped and indexed VCF files, and copy the remapped reads -vcf_res = glob.glob(f"{tmpdirname}/*_vcfs/*vcf") -if vcf_res: - for vcf in vcf_res: - bgzip_cmd = f"bgzip -c {vcf} > {vcf}.gz" - shell(bgzip_cmd) - index_cmd = f"bcftools index {vcf}.gz" - shell(index_cmd) - print(f"Compressed and indexed: {vcf}.gz") - params_variant_files = " ".join([f"{vcf}.gz" for vcf in vcf_res]) - shell( - f"bcftools concat -a -Oz {params_variant_files} | " - f"bcftools annotate --header-lines {snakemake.output.vcf_header} | " - f"bcftools sort -Oz -o {snakemake.output.merged_vcf}" - ) - print(f"Merged, reheadered, and sorted VCF file created: {snakemake.output.merged_vcf}") - # Copy out bam and bai files - bam_res = glob.glob(f"{tmpdirname}/*.bam") - bai_res = glob.glob(f"{tmpdirname}/*.bai") - # print("BAM RES: ", bam_res, bai_res) - shell( - f""" - cp -pr {' '.join(bam_res)} {snakemake.output.bam}; - cp -pr {' '.join(bai_res)} {snakemake.output.bai} - """ - ) -else: - print("No output VCF or BAM files were produced by paraphase, I hope this is what you were expecting, human?") - shell(f"touch {snakemake.output.merged_vcf}")Likely invalid or redundant comment.
Tools
Ruff
126-126: Undefined name
snakemake(F821)
127-127: Undefined name
snakemake(F821)
129-129: Undefined name
snakemake(F821)
137-137: Undefined name
snakemake(F821)
138-138: Undefined name
snakemake(F821)
143-143: Undefined name
snakemake(F821)
|
Formatting issues should be resolved now |
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- bio/paraphase/wrapper.py (1 hunks)
Additional context used
Path-based instructions (1)
bio/paraphase/wrapper.py (2)
Pattern
**/*.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.
Pattern
**/wrapper.py: Do not complain about use of undefined variable calledsnakemake.
Ruff
bio/paraphase/wrapper.py
5-5: Undefined name
snakemake(F821)
6-6: Undefined name
snakemake(F821)
14-14: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
17-17: Undefined name
snakemake(F821)
31-31: Undefined name
snakemake(F821)
32-32: Undefined name
snakemake(F821)
59-59: Undefined name
snakemake(F821)
60-60: Undefined name
snakemake(F821)
63-63: Undefined name
snakemake(F821)
72-72: Undefined name
snakemake(F821)
73-73: Undefined name
snakemake(F821)
80-80: Undefined name
snakemake(F821)
88-88: Undefined name
snakemake(F821)
89-89: Undefined name
snakemake(F821)
91-91: Undefined name
snakemake(F821)
105-105: Undefined name
snakemake(F821)
106-106: Undefined name
snakemake(F821)
134-134: Undefined name
snakemake(F821)
135-135: Undefined name
snakemake(F821)
138-138: Undefined name
snakemake(F821)
147-147: Undefined name
snakemake(F821)
148-148: Undefined name
snakemake(F821)
155-155: Undefined name
snakemake(F821)
Additional comments not posted (5)
bio/paraphase/wrapper.py (5)
1-3: LGTM!The imports are necessary and correctly used.
5-6: LGTM!The logging setup and parameter retrieval are appropriate.
Tools
Ruff
5-5: Undefined name
snakemake(F821)
6-6: Undefined name
snakemake(F821)
9-21: LGTM!The temporary directory usage is appropriate for handling intermediate files.
Tools
Ruff
14-14: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
17-17: Undefined name
snakemake(F821)
11-26: Refactor to avoid code duplication.The code for running
paraphaseand touching the output is duplicated. Consider refactoring to avoid redundancy.Apply this diff to remove the duplicate code:
- with TemporaryDirectory() as tmpdirname: - ### Run paraphase - shell( - f""" - (paraphase --bam {snakemake.input.bam} \ - --reference {snakemake.input.fasta} \ - --out {tmpdirname} \ - {snakemake.params.genome} \ - {extra}) {log} - """, - tmpdirname=tmpdirname, # Pass tmpdirname to the shell environment - ) - shell( - """ - touch {snakemake.output} - """ - )Likely invalid or redundant comment.
Tools
Ruff
14-14: Undefined name
snakemake(F821)
15-15: Undefined name
snakemake(F821)
17-17: Undefined name
snakemake(F821)
28-45: Refactor to avoid code duplication.The code for creating a new VCF header is duplicated. Consider refactoring to avoid redundancy.
Apply this diff to remove the duplicate code:
- ### Create a new VCF header - # Get the paths from Snakemake input and output objects - input_faidx = snakemake.input.faidx - output_vcf_header = snakemake.output.vcf_header - # Open the .fai index file and read lines - with open(input_faidx, "r") as fai_file: - lines = fai_file.readlines() - # Open the output file and write formatted header lines - with open(output_vcf_header, "w") as output: - for line in lines: - contig_id, length = line.split()[:2] # Assuming the first two elements are ID and length - output.write(f"##contig=<ID={contig_id},length={length}>\n")Likely invalid or redundant comment.
Tools
Ruff
31-31: Undefined name
snakemake(F821)
32-32: Undefined name
snakemake(F821)
|
@MagdalenaZZ remember to add the test! |
Hi, it should be allright now? I'll be away for a while, so I hope this us all of the changes needed. |
|
Hi @MagdalenaZZ I do'nt think you've added the test. The tests need to be added to the Lines 909 to 922 in bb4acf9 And you have duplicated your code in |
Paraphase by PacBio, takes HiFi aligned BAMs as input and phases haplotypes for genes of the same family, determines copy numbers and makes phased variant calls.
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,environment.yamlpinning has been updated by runningsnakedeploy pin-conda-envs environment.yamlon a linux machine,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
Bug Fixes