add mtmalign #4339

lrauschning · 2023-11-14T18:04:44Z

This PR adds a module for the structural aligner mTM-align, for use in the multiplesequencealignment pipeline.

PR checklist

Use nf-test instead This reverts commit 1854bd9.

luisas

Super nice Leon!!

I just added a couple of comments - mainly it comes down to allowing the user to use the prefix to change the name of the output file :)

modules/nf-core/mtmalign/align/main.nf

luisas · 2023-12-12T09:27:29Z

modules/nf-core/mtmalign/align/main.nf

+
+    output:
+    tuple val(meta), path("./mTM_result/result.fasta")  , emit: alignment
+    tuple val(meta), path("./mTM_result/result.pdb")    , emit: structure


Suggested change

tuple val(meta), path("./mTM_result/result.pdb") , emit: structure

tuple val(meta), path("./mTM_result/*.pdb") , emit: structure

I don't think this would work – as the inputs are also pdb files, it would also return the input structures.

(mtmalign copies the input structures into the results directory)

Maybe we can then move that file into ${prefix}.pdb ?

I think it's important to have the possibility ti have the output files named correctly

modules/nf-core/mtmalign/align/main.nf

modules/nf-core/mtmalign/align/meta.yml

modules/nf-core/mtmalign/align/tests/main.nf.test

luisas · 2023-12-12T09:43:43Z

modules/nf-core/mtmalign/align/main.nf

+    tag "$meta.id"
+    label 'process_medium'
+
+    conda "bioconda::mtm-align=20220104"


Should we also update it to this?

modules/modules/nf-core/muscle5/super5/main.nf

Line 4 in 0f4f520

conda "${moduleDir}/environment.yml"

true, we should – I'm not sure its documented anywhere, but it is apparently now the preferred way to have conda envs.

Co-authored-by: Luisa Santus <[email protected]>

JoseEspinosa

LGTM! Please fix test and address @luisas's comment and it will be ready to merge.

JoseEspinosa · 2023-12-12T14:35:08Z

modules/nf-core/mtmalign/align/main.nf

+mv ./mTM_result/result.fasta ${prefix}.aln
+mv ./mTM_result/result.pdb ${prefix}.pdb


Suggested change

mv ./mTM_result/result.fasta ${prefix}.aln

mv ./mTM_result/result.pdb ${prefix}.pdb

mv ./mTM_result/result.fasta ${prefix}.aln

mv ./mTM_result/result.pdb ${prefix}.pdb

This reverts commit 83a325a.

lrauschning · 2023-12-12T18:26:45Z

Anybody know what's up with the linting there?

Conflicting process name between environment.yml (mtmalign/align) and meta.yml (mtmalign/align)

seems a bit weird to me.
In the clustalo module, how it was at 0d1bbc0 works (cmp https://github.com/nf-core/modules/blob/master/modules/nf-core/clustalo/align/environment.yml)

modules/nf-core/mtmalign/align/environment.yml

modules/nf-core/mtmalign/align/meta.yml

luisas · 2023-12-13T08:22:51Z

Anybody know what's up with the linting there?
Conflicting process name between environment.yml (mtmalign/align) and meta.yml (mtmalign/align)
seems a bit weird to me. In the clustalo module, how it was at 0d1bbc0 works (cmp https://github.com/nf-core/modules/blob/master/modules/nf-core/clustalo/align/environment.yml)

You can see here how the tags are supposed to be - i made a couple of suggestions that should fix it https://github.com/search?q=repo%3Anf-core%2Fmodules%20clustalo&type=code

modules/nf-core/mtmalign/align/main.nf

Co-authored-by: Luisa Santus <[email protected]>

luisas

This should solve the test error :)

modules/nf-core/mtmalign/align/main.nf

Co-authored-by: Luisa Santus <[email protected]>

@luisas

* commit mtmalign module * comment & add input name * mtmalign test * Revert "mtmalign test" Use nf-test instead This reverts commit 1854bd9. * add nf-tests * fix script path * forgot to delete line from template * removed unnecessary vars * mess around with tests, still not fully working * make licence array instead of string * now runs the nftest * changed tags * now calling untar correctly * added snapshot * fix conflicting name * check fasta instead of snapshot * Apply suggestions from code review Co-authored-by: Luisa Santus <[email protected]> * fix things incorporating @luisas feedback * add environment.yml * whoops, forgot to change env name * change tag name * Revert "change tag name" This reverts commit 83a325a. * try changing in env.yml * Apply suggestions from code review Co-authored-by: Luisa Santus <[email protected]> * remove empty line * add missing bracket * change glob to unique path * Apply suggestions from code review Co-authored-by: Luisa Santus <[email protected]> --------- Co-authored-by: Leon Rauschning <[email protected]> Co-authored-by: Leon Rauschning <[email protected]> Co-authored-by: Luisa Santus <[email protected]>

@luisas

* commit mtmalign module * comment & add input name * mtmalign test * Revert "mtmalign test" Use nf-test instead This reverts commit 1854bd9. * add nf-tests * fix script path * forgot to delete line from template * removed unnecessary vars * mess around with tests, still not fully working * make licence array instead of string * now runs the nftest * changed tags * now calling untar correctly * added snapshot * fix conflicting name * check fasta instead of snapshot * Apply suggestions from code review Co-authored-by: Luisa Santus <[email protected]> * fix things incorporating @luisas feedback * add environment.yml * whoops, forgot to change env name * change tag name * Revert "change tag name" This reverts commit 83a325a. * try changing in env.yml * Apply suggestions from code review Co-authored-by: Luisa Santus <[email protected]> * remove empty line * add missing bracket * change glob to unique path * Apply suggestions from code review Co-authored-by: Luisa Santus <[email protected]> --------- Co-authored-by: Leon Rauschning <[email protected]> Co-authored-by: Leon Rauschning <[email protected]> Co-authored-by: Luisa Santus <[email protected]>

Leon Rauschning and others added 6 commits November 14, 2023 17:53

commit mtmalign module

ae56ffe

comment & add input name

7290cdc

mtmalign test

1854bd9

Merge branch 'master' into mtmalign

cfa5531

Revert "mtmalign test"

60f99b7

Use nf-test instead This reverts commit 1854bd9.

add nf-tests

5f508b6

lrauschning requested a review from a team as a code owner November 15, 2023 15:24

lrauschning requested review from adamrtalbot and removed request for a team November 15, 2023 15:24

Leon Rauschning and others added 11 commits November 15, 2023 16:35

fix script path

99dbdd6

forgot to delete line from template

2bded32

removed unnecessary vars

8127802

mess around with tests, still not fully working

874f088

make licence array instead of string

df3e985

now runs the nftest

f2575a4

changed tags

de4d378

now calling untar correctly

27d3d4a

added snapshot

c76c395

fix conflicting name

a469a17

check fasta instead of snapshot

20fb79a

lrauschning requested review from JoseEspinosa and luisas December 5, 2023 15:05

lrauschning enabled auto-merge December 5, 2023 15:05

lrauschning assigned lrauschning and mathysgrapotte Dec 5, 2023

mathysgrapotte removed their assignment Dec 5, 2023

lrauschning assigned mathysgrapotte Dec 5, 2023

lrauschning added Ready for Review ready to merge labels Dec 6, 2023

lrauschning mentioned this pull request Dec 8, 2023

NF-CORE MODULE: mTMalign nf-core/multiplesequencealign#53

Closed

luisas requested changes Dec 12, 2023

View reviewed changes

luisas reviewed Dec 12, 2023

View reviewed changes

Apply suggestions from code review

afd62a5

Co-authored-by: Luisa Santus <[email protected]>

JoseEspinosa reviewed Dec 12, 2023

View reviewed changes

lrauschning added 6 commits December 12, 2023 18:26

fix things incorporating @luisas feedback

81c579d

add environment.yml

5d5c8e0

whoops, forgot to change env name

0d1bbc0

change tag name

83a325a

Revert "change tag name"

8d1fd5a

This reverts commit 83a325a.

try changing in env.yml

f48a6aa

luisas requested changes Dec 13, 2023

View reviewed changes

modules/nf-core/mtmalign/align/environment.yml Outdated Show resolved Hide resolved

modules/nf-core/mtmalign/align/meta.yml Outdated Show resolved Hide resolved

luisas requested changes Dec 13, 2023

View reviewed changes

modules/nf-core/mtmalign/align/main.nf Show resolved Hide resolved

lrauschning and others added 4 commits December 13, 2023 10:03

Apply suggestions from code review

4929264

Co-authored-by: Luisa Santus <[email protected]>

remove empty line

8f34a78

add missing bracket

c2783cb

change glob to unique path

392b678

luisas approved these changes Dec 13, 2023

View reviewed changes

modules/nf-core/mtmalign/align/main.nf Outdated Show resolved Hide resolved

modules/nf-core/mtmalign/align/main.nf Outdated Show resolved Hide resolved

Apply suggestions from code review

a620af5

Co-authored-by: Luisa Santus <[email protected]>

lrauschning added this pull request to the merge queue Dec 14, 2023

Merged via the queue into nf-core:master with commit e09a91f Dec 14, 2023

lrauschning deleted the mtmalign branch December 14, 2023 10:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add mtmalign #4339

add mtmalign #4339

lrauschning commented Nov 14, 2023

luisas left a comment

luisas Dec 12, 2023

lrauschning Dec 12, 2023

lrauschning Dec 12, 2023

luisas Dec 13, 2023

luisas Dec 12, 2023

lrauschning Dec 12, 2023

JoseEspinosa left a comment

JoseEspinosa Dec 12, 2023

lrauschning commented Dec 12, 2023

luisas commented Dec 13, 2023

luisas left a comment

	tuple val(meta), path("./mTM_result/result.pdb") , emit: structure
	tuple val(meta), path("./mTM_result/*.pdb") , emit: structure

		mv ./mTM_result/result.fasta ${prefix}.aln
		mv ./mTM_result/result.pdb ${prefix}.pdb

add mtmalign #4339

add mtmalign #4339

Conversation

lrauschning commented Nov 14, 2023

PR checklist

luisas left a comment

Choose a reason for hiding this comment

luisas Dec 12, 2023

Choose a reason for hiding this comment

lrauschning Dec 12, 2023

Choose a reason for hiding this comment

lrauschning Dec 12, 2023

Choose a reason for hiding this comment

luisas Dec 13, 2023

Choose a reason for hiding this comment

luisas Dec 12, 2023

Choose a reason for hiding this comment

lrauschning Dec 12, 2023

Choose a reason for hiding this comment

JoseEspinosa left a comment

Choose a reason for hiding this comment

JoseEspinosa Dec 12, 2023

Choose a reason for hiding this comment

lrauschning commented Dec 12, 2023

luisas commented Dec 13, 2023

luisas left a comment

Choose a reason for hiding this comment