Improve backsub input validation and testing #83
Conversation
Co-authored-by: Krešimir Beštak <kbestak@gmail.com>
Co-authored-by: Krešimir Beštak <kbestak@gmail.com>
Co-authored-by: Krešimir Beštak <kbestak@gmail.com>
Co-authored-by: Krešimir Beštak <kbestak@gmail.com>
There was a problem hiding this comment.
Hey @jmuhlich, thank you for making time to add the changes. This is a very nice and elegant approach that allows the markersheet to be expanded in the future in a very straightforward and logical manner.
I've only noticed that if remove values are given and backsub is set to false, a warning is given, but the pipeline fails on MCQUANT due to the remove marker logic on the nextflow side. My suggestion would be to throw an error for this case as you can see above.
Looks good to me! 🚀
| def backsub_columns = ['exposure', 'background', 'remove'] | ||
| if (markersheet_data*.subMap(backsub_columns)*.collect{ c, v -> v != null }.flatten().any()) { |
There was a problem hiding this comment.
If the remove column is provided, params.backsub is false, if just a warning is passed, MCQUANT would fail due to channel removal logic. Adding an error targeting this case
| def backsub_columns = ['exposure', 'background', 'remove'] | |
| if (markersheet_data*.subMap(backsub_columns)*.collect{ c, v -> v != null }.flatten().any()) { | |
| def backsub_columns = ['exposure', 'background'] | |
| if (markersheet_data*.subMap('remove')*.collect{ c, v -> v != null }.flatten().any()) { | |
| error("The remove column is present in the marker sheet, but params.backsub is set to false. Please set params.backsub to true or remove TRUE values from the remove column.") | |
| } | |
| else if (markersheet_data*.subMap(backsub_columns)*.collect{ c, v -> v != null }.flatten().any()) { |
| //assert function.stdout*.contains("Subtraction will NOT be performed").any() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
If adding error for params.backsub = false and remove values added
| test("Error if backsub false but remove is provided") { | |
| function "validateInputMarkersheet" | |
| when { | |
| params { | |
| backsub = true | |
| } | |
| function { | |
| """ | |
| input[0] = [ | |
| [marker_name:"M1", cycle_number:1, channel_number:1, remove: true], | |
| ] | |
| """ | |
| } | |
| } | |
| then { | |
| assert function.failed | |
| assert function.stdout*.contains("The remove column is present in the marker sheet").any() | |
| } | |
| } | |
| """ | ||
| input[0] = [ | ||
| [marker_name:"M1", cycle_number 96F8 :1, channel_number:1, exposure: 100], | ||
| [marker_name:"M2", cycle_number:1, channel_number:2, remove: true], |
There was a problem hiding this comment.
If adding error for params.backsub = false and remove values added since this would now fail istead of warn
| [marker_name:"M2", cycle_number:1, channel_number:2, remove: true], | |
| [marker_name:"M2", cycle_number:1, channel_number:2, background:"M1"], |
Rework of #82. Original features retained:
removecolumn non-required.removefrom markersheet before generating input markersheet for quantification so that markersheet rows match up with image channels.Updates relative to the original PR:
metawith a default value ofnull. Update all relevant places in the code and tests to use these maps instead of the old lists.PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).