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: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"], |
When backsub is not being used, we now properly ignore the "remove" channel meta property.
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 3.2.0. For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation. |
|
Thanks for the suggested changes, but I'm afraid that solution defeats the design goal of allowing the user to enable/disable backsub without also needing to change their marker sheet. I implemented an alternative fix which corrects the quantification markersheet generation, not removing channels if params.backsub is false. |
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>).