Add wittyer as module #5171

famosab · 2024-03-18T11:00:15Z

PR checklist

Closes #4739 , relates to nf-core/variantbenchmarking#10

According to slack discussion, the docker image still needs to be added to the nf-core registry.

nvnieuwk

It's looking very promising already!

nvnieuwk · 2024-03-18T15:24:14Z

modules/nf-core/wittyer/main.nf

+    mv bench/Wittyer.Stats.json ${prefix}.Stats.json
+    mv bench/*.vcf.gz ${prefix}.eval.vcf.gz
+    mv bench/*.vcf.gz.tbi ${prefix}.eval.vcf.gz.tbi


Could you remove any custom extensions that are not file extensions? This should be done to make all filenames as customizable as possible :)

I think eval was added to show that this vcf is the result of the benchmarking comparison. Should that be captured via prefix then?

Yes that should be added via the prefix. It could also be a good idea to add a simple check to see if the output filename isn't the same as the input files (otherwise Nextflow will give some weird errors) => See https://nf-co.re/docs/contributing/modules#naming-conventions part 6 for an example

but the name is generated through wittyer, I think it makes sense to keep original extensions.

eval is already exist in original output, only prefix is being added here to the name. I would prefer it in this way actually.

I changed it now and added the if statements to make sure we don't run into Nextflow Errors.

But I was not adding the extension by myself :D it is the standard output by the tool.

This type of names are important for traceability with standard outputs of the tool. So I would really prefer to keep is as it is.

But the eval is only added when you move the file from the output directory?

no, I am copying the result here. eval is in standard output.

modules/nf-core/wittyer/main.nf

famosab · 2024-03-19T09:47:47Z

modules/nf-core/wittyer/tests/main.nf.test

+        then {
+            assertAll(
+                { assert process.success },
+                { assert snapshot(process.out).match() }


Do we need more assertions here?

yes, per output

modules/nf-core/wittyer/environment.yml

modules/nf-core/wittyer/meta.yml

modules/nf-core/wittyer/tests/main.nf.test

Co-authored-by: Nicolas Vannieuwkerke <[email protected]>

nvnieuwk

Almost there!

nvnieuwk · 2024-03-19T13:45:50Z

modules/nf-core/wittyer/main.nf

+
+    input:
+    // use stageAs to allow comparing the same vcfs against each other
+    tuple val(meta), path(query_vcf, stageAs: "query.vcf.gz"), path(query_vcf_index, stageAs: "query.vcf.gz.tbi"), path(truth_vcf, stageAs: "truth.vcf.gz"), path(truth_vcf_index, stageAs: "truth.vcf.gz.tbi"), path(bed)


Don't use stageAs here, it's easier to debug modules if they retain the original filenames :)

I wanted to test truth and query with the same files. Because I find it easier to check if my benchmarking workflow works when I can compare a file against itself. So if I remove it here it would need to give them different names?

Yeah but at the cost of making it harder on yourself when you want to actually use the process? Can't you make a copy of the query file and rename it before running the pipeline? :p

I'll change it and adapt the test accordingly

nvnieuwk · 2024-03-20T08:49:49Z

Can you also exclude the conda test in .github/workflows/test.yml? You'll find a list of modules that have their conda test excluded. Please add this module there so you won't merge any failing tests :)

nvnieuwk

LGTM!

* feat: add wittyer as module * fix: adjust output and version * fix: copy output and script from local * feat: fill in in meta file * fix: remove config file input * fix: run prettier * feat: explain how to create docker image * fix: README for Docker image * fix: modify environment file * fix: stub and file extensions * fix: remove custom extensions * feat: add test file - failing * fix: correct input for tests * fix: licence * fix: container name * fix: finally correct container * fix syntax errors * normal test works, stub fails * fix expected outputs * fix: mention making image public * fix add stageAs, use dotnet * run prettier * [automated] Fix linting with Prettier * add comment for run * fix trailing whitespace * add dependencies to fix linting * fix harmonize inputs in meta and main * fix: run prettier * [automated] Fix linting with Prettier * fix index names * fix version to stub_version in stub test Co-authored-by: Nicolas Vannieuwkerke <[email protected]> * extend tests to capture as much as possible * test with bed file * fix: remove stageAs * fix tests for stageAs removal * remove '' in meta file * exclude conda test for wittyer --------- Co-authored-by: nf-core-bot <[email protected]> Co-authored-by: Nicolas Vannieuwkerke <[email protected]>

* master: Added contrast limited adaptive histogram equalization module (nf-core#5268) Leviosam2 index (nf-core#5316) Add subworkflow mapAD (nf-core#5239) Remove AMRFinderPlus DB update on each invocation (nf-core#5232) Revert "add paths in output directive in cellranger cout module" (nf-core#5306) Revert "update kallistobustools count output list" (nf-core#5307) Igv reports (nf-core#5263) Remove unnecessary .view() in subworkflows/nf-core/vcf_phase_shapeit5 & tests/modules/nf-core/shapeit5/ligate/main.nf (nf-core#5280) Add wittyer as module (nf-core#5171) Bamstats (nf-core#4474) gatk4_asereadcounter add updated meta and nf-tests (nf-core#5164) 4557 new module kaijumergeoutputs + stub Kraken2/Kraken2 (nf-core#5249) add cram/index support to bwamem2 (nf-core#5248) Add README to modules build with Docker (nf-core#4935) update paths for VEP (nf-core#5281) nf-test bases2fastq (nf-core#5272) Add module seqfu/stats (nf-core#5275)

* master: Added contrast limited adaptive histogram equalization module (nf-core#5268) Leviosam2 index (nf-core#5316) Add subworkflow mapAD (nf-core#5239) Remove AMRFinderPlus DB update on each invocation (nf-core#5232) Revert "add paths in output directive in cellranger cout module" (nf-core#5306) Revert "update kallistobustools count output list" (nf-core#5307) Igv reports (nf-core#5263) Remove unnecessary .view() in subworkflows/nf-core/vcf_phase_shapeit5 & tests/modules/nf-core/shapeit5/ligate/main.nf (nf-core#5280) Add wittyer as module (nf-core#5171) Bamstats (nf-core#4474) gatk4_asereadcounter add updated meta and nf-tests (nf-core#5164) 4557 new module kaijumergeoutputs + stub Kraken2/Kraken2 (nf-core#5249) add cram/index support to bwamem2 (nf-core#5248) Add README to modules build with Docker (nf-core#4935) update paths for VEP (nf-core#5281) nf-test bases2fastq (nf-core#5272) Add module seqfu/stats (nf-core#5275) Added gt/gff3validator (nf-core#4718) Added liftoff (nf-core#5209)

* feat: add wittyer as module * fix: adjust output and version * fix: copy output and script from local * feat: fill in in meta file * fix: remove config file input * fix: run prettier * feat: explain how to create docker image * fix: README for Docker image * fix: modify environment file * fix: stub and file extensions * fix: remove custom extensions * feat: add test file - failing * fix: correct input for tests * fix: licence * fix: container name * fix: finally correct container * fix syntax errors * normal test works, stub fails * fix expected outputs * fix: mention making image public * fix add stageAs, use dotnet * run prettier * [automated] Fix linting with Prettier * add comment for run * fix trailing whitespace * add dependencies to fix linting * fix harmonize inputs in meta and main * fix: run prettier * [automated] Fix linting with Prettier * fix index names * fix version to stub_version in stub test Co-authored-by: Nicolas Vannieuwkerke <[email protected]> * extend tests to capture as much as possible * test with bed file * fix: remove stageAs * fix tests for stageAs removal * remove '' in meta file * exclude conda test for wittyer --------- Co-authored-by: nf-core-bot <[email protected]> Co-authored-by: Nicolas Vannieuwkerke <[email protected]>

feat: add wittyer as module

ff7fea5

famosab self-assigned this Mar 18, 2024

famosab added 5 commits March 18, 2024 14:08

fix: adjust output and version

03da21d

fix: copy output and script from local

699666c

feat: fill in in meta file

1efcfe3

fix: remove config file input

9b5c0c7

fix: run prettier

b79ee74

nvnieuwk reviewed Mar 18, 2024

View reviewed changes

famosab added 6 commits March 18, 2024 17:08

feat: explain how to create docker image

cbb2e0b

fix: README for Docker image

c974cbd

fix: modify environment file

e0f5f69

fix: stub and file extensions

213d6fa

fix: remove custom extensions

80e352e

feat: add test file - failing

7babe7a

famosab commented Mar 19, 2024

View reviewed changes

modules/nf-core/wittyer/main.nf Outdated Show resolved Hide resolved

famosab commented Mar 19, 2024

View reviewed changes

famosab and others added 14 commits March 19, 2024 11:16

fix: correct input for tests

e702a20

fix: licence

522f72e

fix: container name

7d06402

fix: finally correct container

fdf993d

fix syntax errors

c07bd11

normal test works, stub fails

d6e428b

fix expected outputs

3f7e016

fix: mention making image public

bf138d6

fix add stageAs, use dotnet

d205ddd

run prettier

e416458

[automated] Fix linting with Prettier

a6564b6

add comment for run

a205762

fix trailing whitespace

ae11718

add dependencies to fix linting

32081bc

famosab and others added 2 commits March 19, 2024 14:09

fix version to stub_version in stub test

b2a8c10

Co-authored-by: Nicolas Vannieuwkerke <[email protected]>

extend tests to capture as much as possible

9f908f8

famosab requested a review from nvnieuwk March 19, 2024 13:25

Merge branch 'master' into wittyer

4528b41

famosab added the new module Adding a new module label Mar 19, 2024

famosab marked this pull request as ready for review March 19, 2024 13:29

famosab requested a review from a team as a code owner March 19, 2024 13:29

test with bed file

fd4c307

famosab requested a review from kubranarci March 19, 2024 13:36

nvnieuwk reviewed Mar 19, 2024

View reviewed changes

famosab added 2 commits March 20, 2024 09:35

fix: remove stageAs

6aac9e8

fix tests for stageAs removal

67fa022

famosab added 2 commits March 20, 2024 10:00

remove '' in meta file

e46dbd3

exclude conda test for wittyer

682c539

famosab requested a review from nvnieuwk March 20, 2024 09:09

Merge branch 'master' into wittyer

79aac08

nvnieuwk approved these changes Mar 20, 2024

View reviewed changes

famosab added this pull request to the merge queue Mar 20, 2024

Merged via the queue into nf-core:master with commit 61f2ea5 Mar 20, 2024
10 checks passed

famosab deleted the wittyer branch March 20, 2024 09:38

famosab mentioned this pull request Mar 20, 2024

wittyer module nf-core/variantbenchmarking#10

Closed

famosab mentioned this pull request Jul 8, 2024

new module: dorado #5455

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wittyer as module #5171

Add wittyer as module #5171

famosab commented Mar 18, 2024 •

edited

Loading

nvnieuwk left a comment

nvnieuwk Mar 18, 2024

famosab Mar 18, 2024

nvnieuwk Mar 18, 2024

kubranarci Mar 19, 2024

kubranarci Mar 19, 2024

famosab Mar 19, 2024

kubranarci Mar 19, 2024

kubranarci Mar 19, 2024

famosab Mar 19, 2024

kubranarci Mar 19, 2024

famosab Mar 19, 2024

kubranarci Mar 19, 2024

nvnieuwk left a comment

nvnieuwk Mar 19, 2024

famosab Mar 19, 2024

nvnieuwk Mar 19, 2024

famosab Mar 19, 2024

nvnieuwk commented Mar 20, 2024

nvnieuwk left a comment

Add wittyer as module #5171

Add wittyer as module #5171

Conversation

famosab commented Mar 18, 2024 • edited Loading

PR checklist

nvnieuwk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvnieuwk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvnieuwk commented Mar 20, 2024

nvnieuwk left a comment

Choose a reason for hiding this comment

famosab commented Mar 18, 2024 •

edited

Loading