Add correlation of samples on peaks workflow#1300
Conversation
lldelisle
left a comment
There was a problem hiding this comment.
Thanks @OEAdebayo for putting this workflow on IWC. I added suggestions to the README. Please if you modify your README, think about modifying also the README in the .ga file.
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Co-authored-by: Lucille Delisle <lucille.delisle@unige.ch>
Test Results (powered by Planemo)Test Summary
Passed Tests
|
Test Results (powered by Planemo)Test Summary
Passed Tests
|
|
Fixed the lint error |
|
@mvdbeek Do you want to review? |
|
Ah I forgot the CODEOWNERS. @mvdbeek how does it works for new members? |
There was a problem hiding this comment.
Thanks a lot for this contribution, @OEAdebayo! The workflow design is solid — inputs come in as collections, the genome is exposed as a parameter rather than pinned, and the test file is set up correctly with output labels that match the workflow. The comments below are all small fixes; nothing structural.
(Review assembled with Claude Code on my behalf, checked against the IWC reviewer checklist.)
Needs fixing
1. Annotation phrasing
The checklist asks that the annotation start with This workflow does/runs/performs … to generate/analyze/…. Currently it starts with "Takes a list of peak files…". Something like:
This workflow computes read coverage of BAM/CRAM samples over a merged set of peak regions to generate a pairwise correlation plot between samples.
(There's also a double space in "sorts and merges".)
2. Changelog date is malformed
## [0.1] 20206-07-06 -> ## [0.1] - 2026-07-06
Note the 20206, and the missing - separator that the rest of the repo uses (compare workflows/epigenetics/chipseq-pe/CHANGELOG.md). There are also some trailing-whitespace lines and no newline at end of file.
3. Output label typo
deeptool raw counts should be deepTools raw counts — "deeptool" is a typo for the tool suite name. Please make the same edit in correlation-of-samples-on-peaks-tests.yml so the labels stay in sync. Capitalising the other two labels (Correlation plot, deepTools coverage matrix) would read a bit better too, same sync caveat.
4. Step labels and annotations
- Step 2 annotation:
"This takes the BAM /CRAM formated peak files "— "formated" -> "formatted", stray space in "BAM /CRAM", and these are alignment files rather than peak files. Trailing space can go as well. - Step 3 annotation: "This step concatenate multiple datasets" -> "concatenates".
- Step 6 label:
multiBam summary->multiBamSummary(the actual tool name). - Steps 6 and 7 have no annotation while every other step does — worth filling in.
5. README
The content covers inputs, the parameter, processing, and all three outputs, which is what we want. Small things:
- Title
# correlation of samples on peaks:-># Correlation of samples on peaks(title case, no trailing colon). - "Inputs values" -> "Input values".
workflows/epigenetics/consensus-peaks/already merges peaks across samples. The checklist asks that a README explain how it differs from a similar existing workflow — a sentence on when to prefer this one (correlation QC across samples) would help users pick.
Passing
- .dockstore.yml: present, valid, and all three authors carry ORCIDs matching the
creatorblock in the.gaexactly. - Folder name:
correlation-of-samples-on-peaks— lowercase, dashes, reads correctly. - Input/output labels: human readable, no underscores, and all three
workflow_outputslabels match the test file. - Large files: largest test file is 38 KB, so no Zenodo upload needed.
Happy to re-review once these are addressed — thanks again!
|
You can already add anyone to the CODEOWNERS, and then we add @OEAdebayo as a maintainer. |
new galaxy workflow correlation-of-samples-on-peaks