feat: more output files labelled temp #9
Conversation
WalkthroughIntermediate outputs in preparation and docking Snakemake rules were marked temporary by wrapping rule Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🔇 Additional comments (9)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
workflow/rules/docking.smk (2)
66-71: Critical syntax error: Missing closing parenthesis for temp().The
temp()call is missing its closing parenthesis. Additionally, the comma after line 71 suggests this might have been intended as a named output, but the output name is missing.Apply this diff to fix the syntax:
output: - temp(path.join( + temp( + path.join( "docking", "{receptorID}", "{dataset}", "{database}_{dataset}_{name}_{i}.txt", - ), + ) + ),Note: If this was intended to be a named output, you'll need to add the name, e.g.,
lig=temp(path.join(...)).
136-146: The merged docking output is orphaned and will be deleted.The verification confirms the original concern:
mergeDockingproduces temporary output atdocking/{receptorID}/{receptorID}_{database}_{dataset}_{name}.pdbqt.gz, but no downstream rules consume this path. The workflow's final outputs (inresults/) are generated independently by rules inanalyse.smk, not from the merged docking files. Withtemp()marking, these merged results will be automatically deleted after the workflow completes.Either remove the
temp()wrapper if users need to keep merged docking results, or delete themergeDockingrule entirely if it serves no purpose.workflow/rules/preparation.smk (1)
252-260: Removetemp()wrapper from prepareDocking output or ensure this rule's output is actually consumed.Verification confirms this temporary file is never consumed downstream. The
docking.smkruleprepare_docking_localtakes its receptor input directly fromrules.makeReceptorPDBQT.output, not from thereceptor/{receptorID}.txtfile created byprepareDocking. Since no downstream rule references this output, marking it as temporary will cause Snakemake to delete it prematurely. Either remove thetemp()wrapper if this is a persistent metadata file, or remove the rule entirely if it's unused.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
workflow/rules/docking.smk(3 hunks)workflow/rules/preparation.smk(3 hunks)
🔇 Additional comments (2)
workflow/rules/preparation.smk (2)
120-133: LGTM! Appropriate use of temp() for intermediate receptor files.Marking the receptor PDBQT file as temporary is a good practice for reducing disk space usage. Snakemake will automatically preserve this file until all downstream rules (like
prepareDockingand rules indocking.smk) have consumed it.
214-239: LGTM! Grid geometry files appropriately marked as temporary.The grid geometry file is an intermediate artifact that can be safely removed after dependent rules consume it. This change aligns well with the PR's objective of reducing disk space usage.
| os.makedirs(outdir, exist_ok=True) | ||
|
|
||
| shutil.copy(input.ligands, output.lig) | ||
| shutil.copy(input.ligands, output[0]) |
There was a problem hiding this comment.
Output reference change is correct, but breaks downstream rule.
The change from output.lig to output[0] is correct since temp() returns a positional output. However, this breaks the docking rule at line 94, which still references rules.prepare_docking_ligand.output.lig.
Apply this diff to fix the broken reference:
input:
rec=rules.prepare_docking_local.output[0],
geo=rules.prepare_docking_local.output[1],
- lig=rules.prepare_docking_ligand.output.lig,
+ lig=rules.prepare_docking_ligand.output[0],Alternatively, if you prefer named outputs for clarity, modify the output definition at lines 66-71 to:
output:
lig=temp(
path.join(
"docking",
"{receptorID}",
"{dataset}",
"{database}_{dataset}_{name}_{i}.txt",
)
),And keep line 94 as is. This approach provides better readability.
Summary by CodeRabbit