Dedup subworkflow name

[SPPearce]

I propose that we rename the bam_dedup_stats_samtools_umitools subworkflow to be just bam_dedup_umitools. Various other subworkflows generate stats using samtools, without mentioning it in the name of the subworkflow.

I think the main tool should be mentioned in the title, not the auxiliary tools.

[SPPearce]

Also edit tests/config/pytest_modules.yml

Ah! Didn't spot that it was listed starting with subworkflows in here, checked under bam. Should have grepped of course...

[edmundmiller]

There's also bam_sort_stats_samtools? What do we want to do about that one?

[matthdsm]

That's a toughy since it's both samtools...

[SPPearce]

There's also bam_sort_stats_samtools? What do we want to do about that one?

That one does actually sort the bam and the gets the stats using samtools, so I'm less fussed about that.

[matthdsm]

I'd even consider dropping stats from all the various workflows. I don't see much value in running it a million times when using subworkflows. I would much rather have 1 qc workflow/step I could put where I want, without being forced into running stats every time.

[matthdsm]

⏫ at some point we're just duplicating code IMO

[edmundmiller]

Yeah, that's the tough part with subworkflows, it's opinionated. We can't get down to something atomic like the modules, it's just going to have to be community consensus. But without the stats, you're just basically running UMITOOLS_DEDUP :shrug:

[nvnieuwk]

Maybe we could add an option to each subworkflow that defines whether the subworkflow runs the stats or not?

[matthdsm]

I think we should get to a point where’s there guidelines as to what warrants a subworkflow so we dont get a bunch of tool x + stats or whatever. Op 17 okt. 2022 om 16:22 heeft nvnieuwk @.***> het volgende geschreven:



Maybe we could add an option to each subworkflow that defines whether the subworkflow runs the stats or not?

— Reply to this email directly, view it on GitHubhttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnf-core%2Fmodules%2Fpull%2F2318%23issuecomment-1280941748&data=05%7C01%7CMatthias.DeSmet%40ugent.be%7C412726722bd349b2742608dab04b071e%7Cd7811cdeecef496c8f91a1786241b99c%7C1%7C0%7C638016133526368131%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=LBKS5q65b8WPL%2Ft9htFipkpBKbK%2B1PP97Z3o9nyFgF0%3D&reserved=0, or unsubscribehttps://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAC2NHEEYE7E4RB6S7G57MM3WDVOKNANCNFSM6AAAAAARHBE63Y&data=05%7C01%7CMatthias.DeSmet%40ugent.be%7C412726722bd349b2742608dab04b071e%7Cd7811cdeecef496c8f91a1786241b99c%7C1%7C0%7C638016133526525036%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qGyKlA21%2Blb9SOEzvX2uWPDd%2Fa7SXdrp5S9AUROAB4U%3D&reserved=0. You are receiving this because you commented.Message ID: @.***>

[nvnieuwk]

Yep that would be useful :)

[drpatelh]

Agree we should maybe have an option to switch the stats off. Still think we should keep it in the name though otherwise what distinguishes it from another subworkflow that doesn't include this at all? We should be as explicit as possible about what the subworkflow is actually doing so it's easier to find, use and reuse if required with other tools that could be drop in replacements. Happy to have another discussion about this next week maybe on a maintainers call?

[SPPearce]

bam_dedup_stats_samtools_umitools sounds like it might be using samtools markdup, whereas the actual important part of the name is umitools.

[matthdsm]

@drpatelh , @SPPearce What's the status of this PR? Are we good to merge or should we convert to draft and/or close? We're cleaning up open PR's so we have nice place to start the upcoming Hackathon!

Cheers Matthias - nf-core maintainers

[SPPearce]

I'm still of the opinion that we should rename this subworkflow, and think that the other subworkflow names in the recent months are on the side of renaming it.

[matthdsm]

We might want to merge https://github.com/nf-core/modules/pull/2979 before we proceed though

[SPPearce]

Yes, and probably #2971 as well.

[maxulysse]

What's happening with this PR, should we drop it and restart from scratch?

[SPPearce]

Probably best to close this one, yes. Are we renaming the subworkflow then?

[maxulysse]

Are we renaming the subworkflow then?

Let's make an issue for that, and discuss there, and then if needed we can have a PR