Skip to content

fix: Support both reference.dict and reference.fasta.dict in GRIDSS wrappers #4426

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

fix: Support both reference.dict and reference.fasta.dict in GRIDSS wrappers #4426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7701822
Support both reference.dict and reference.fasta.dict in GRIDSS wrappers
RitzelC Aug 19, 2025
2819e1c
Small tweaks
fgvieira Aug 20, 2025
6605296
Use f-strings for readability
fgvieira Aug 20, 2025
dc3b3ef
Merge branch 'snakemake:master' into gridss_preprocess_fix_endings
RitzelC Aug 25, 2025
56c2d5f
Merge branch 'master' into gridss_preprocess_fix_endings
fgvieira Sep 5, 2025
45f598b
Merge branch 'master' into gridss_preprocess_fix_endings
johanneskoester Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bio/gridss/assemble/test/Snakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ reference_index_endings = (".amb",".ann", ".bwt", ".pac", ".sa", ".gridsscache",

rule gridss_assemble:
input:
bams=expand("mapped/{sample}.bam", sample=samples),
bais=expand("mapped/{sample}.bam.bai", sample=samples),
bam=expand("mapped/{sample}.bam", sample=samples),
bai=expand("mapped/{sample}.bam.bai", sample=samples),
reference="reference/genome.fasta",
dictionary="reference/genome.dict",
indices=multiext("reference/genome.fasta", *reference_index_endings),
Comment on lines 38 to 40
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reference="reference/genome.fasta",
dictionary="reference/genome.dict",
indices=multiext("reference/genome.fasta", *reference_index_endings),
ref="reference/genome.fasta",
dict="reference/genome.dict",
idx=multiext("reference/genome.fasta", *reference_index_endings),

Expand Down
0 ...ridss/assemble/test/reference/genome.dict → ...assemble/test/reference/genome.fasta.dict
View file
Open in desktop
File renamed without changes.
8 changes: 3 additions & 5 deletions bio/gridss/assemble/wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@
)
)

dictionary = path.splitext(reference)[0] + ".dict"
dictionary = reference + ".dict"
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
Comment on lines +34 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
dictionary = reference + ".dict"
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."

)
Comment on lines +34 to 38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Support both reference.fa.dict and reference.dict (and an explicit input) to avoid regressions

The current check only accepts reference + ".dict". That breaks setups that still produce reference.dict, which this PR aims to support. Recommend trying both naming conventions and, if present, honoring an explicit snakemake.input.dictionary (aligning with call/preprocess wrappers).

Apply this diff to make the check robust and backward-compatible:

-dictionary = reference + ".dict"
-if not path.exists(dictionary):
-    raise ValueError(
-        f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
-    )
+dictionary = snakemake.input.get("dictionary")
+if dictionary:
+    if not path.exists(dictionary):
+        raise ValueError(
+            f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
+        )
+else:
+    candidates = [f"{reference}.dict", f"{path.splitext(reference)[0]}.dict"]
+    if not any(path.exists(d) for d in candidates):
+        raise ValueError(
+            "Reference dictionary not found. Looked for: {}. Please create it (e.g., with Picard CreateSequenceDictionary).".format(
+                ", ".join(candidates)
+            )
+        )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
dictionary = reference + ".dict"
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
)
# before this block, reference is defined above
# replace the original dictionary existence check with:
dictionary = snakemake.input.get("dictionary")
if dictionary:
if not path.exists(dictionary):
raise ValueError(
f"{dictionary} missing. Please make sure the reference dictionary was properly created. "
"This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
)
else:
candidates = [f"{reference}.dict", f"{path.splitext(reference)[0]}.dict"]
if not any(path.exists(d) for d in candidates):
raise ValueError(
"Reference dictionary not found. Looked for: {}. Please create it (e.g., with Picard CreateSequenceDictionary).".format(
", ".join(candidates)
)
)
🤖 Prompt for AI Agents 
In bio/gridss/assemble/wrapper.py around lines 34 to 38, the current check only
looks for reference + ".dict" which breaks setups that use reference.dict or
provide an explicit snakemake.input.dictionary; update the logic to first prefer
and honor an explicit snakemake.input.dictionary if present, then check for both
reference + ".fa.dict" and reference + ".dict" (or other common variants) and
use the first one found, raising the missing-file ValueError only if none of
these exist; ensure the chosen dictionary path is assigned to the same variable
used downstream.


shell(
Expand All @@ -45,6 +43,6 @@
"--threads {snakemake.threads} " # Threads
"--workingdir {snakemake.params.workingdir} " # Working directory
"--assembly {snakemake.output.assembly} " # Assembly output
"{snakemake.input.bams} "
"{snakemake.input.bam} "
"{extra}) {log}"
)
0 bio/gridss/call/test/reference/genome.dict → ...dss/call/test/reference/genome.fasta.dict
View file
Open in desktop
File renamed without changes.
6 changes: 2 additions & 4 deletions bio/gridss/call/wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@
)
)

dictionary = path.splitext(reference)[0] + ".dict"
#dictionary = path.splitext(reference)[0] + ".dict"
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
Comment on lines +34 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#dictionary = path.splitext(reference)[0] + ".dict"
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."

)

shell(
Expand Down
0 ...dss/preprocess/test/reference/genome.dict → ...eprocess/test/reference/genome.fasta.dict
View file
Open in desktop
File renamed without changes.
6 changes: 2 additions & 4 deletions bio/gridss/preprocess/wrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,10 @@
)
)

dictionary = path.splitext(reference)[0] + ".dict"
# dictionary = path.splitext(reference)[0] + ".dict" --> leads to incorrect paths (reference.dict), which is incompatible with setup reference (reference.fa.dict)
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing or wrong naming (reference.fa.dict). Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."
Comment on lines +34 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# dictionary = path.splitext(reference)[0] + ".dict" --> leads to incorrect paths (reference.dict), which is incompatible with setup reference (reference.fa.dict)
if not path.exists(dictionary):
raise ValueError(
"{dictionary}.dict missing. Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard".format(
dictionary=dictionary
)
f"{dictionary} missing or wrong naming (reference.fa.dict). Please make sure the reference dictionary was properly created. This can be accomplished for example by CreateSequenceDictionary.jar from Picard."

)

shell(
Expand Down
Loading