Added stub and nf-test for merquryfk/merquryfk #5632
Conversation
|
|
|
I have no idea really. Best ask on nf-core. I see also that some of the process selectors aren't matched, but I don't think that has anything to do with it. |
There was a problem hiding this comment.
If there is no conda package I would just remove the environment.yml file. You can suppress the conda tests in the by adding it here: https://github.com/nf-core/modules/blob/master/.github/workflows/test.yml
|
|
||
| touch "${prefix}.qv" | ||
|
|
||
| echo \\ |
There was a problem hiding this comment.
where is this echoed to? I think for the stub test it is enough to just create the expected output files via touch
There was a problem hiding this comment.
This is echoed to standard output (.command.log) and can be helpful while debugging to know the original command that is executed without -stub.
| script "../../../fastk/fastk" | ||
| process { | ||
| """ | ||
| input[0] = [ |
There was a problem hiding this comment.
Can you update the file paths according as shown here: https://github.com/nf-core/tools/pull/2985/files
| [ id:'test', single_end:true ], [] | ||
| ] | ||
|
|
||
| input[0] = FASTK_FASTK.out.hist |
There was a problem hiding this comment.
I think it is enough to collect them all in a list given top input[0] but it is quite readable.
| """ | ||
| assembly = [ | ||
| [ id:'test', single_end:true ], // meta map | ||
| file(params.test_data['homo_sapiens']['genome']['genome_fasta'], checkIfExists: true) |
| } | ||
| } | ||
|
|
||
| test("test_merquryfk_merquryfk_png") { |
There was a problem hiding this comment.
Can you give a name that adheres more to the guidelines like (human - fasta)
| } | ||
| } | ||
|
|
||
| test("multiple_minimal_stub") { |
There was a problem hiding this comment.
I am not sure: Why do you need two different stub tests? 🤔
There was a problem hiding this comment.
One runs with haplotigs and the other without it.
|
Thank you @mahesh-panchal and @famosab for the feedback MERQURY.FK is not a well maintained tool. Even if we find out and fix this FASTK segmentation issue, it does not work in trio mode. I have tested that elsewhere. As suggested by @mahesh-panchal slack post, I agree we should be using MERQURY itself. See PR: #5657 I would like to close this PR. |
|
@GallVp I think you'll need to deprecate the module (https://nf-co.re/docs/contributing/modules#deprecating-a-module) then and create an issue for a new one. But that seems sensible to me. |
|
I agree with deprecating this module too. It's kind of a shame, because the speed up and memory profile were much better, but really it would have been nicer if that was contributed directly back to the Merqury rather than a new unversioned unmaintained tool made. It was also nice that KATGC and SMUDGEPLOT were bundled here too. |
|
Good idea. But deprecating the module requires going through the CI, which is failing? |
|
True. Closing then seems the best. I'm still not sure what the issue is with the self hosted runners but they're not set up in the same way as Githubs. |
PR checklist
Closes #XXX
versions.ymlfile.labelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile condanf-core subworkflows test <SUBWORKFLOW> --profile dockernf-core subworkflows test <SUBWORKFLOW> --profile singularitynf-core subworkflows test <SUBWORKFLOW> --profile conda