Skip to content

mapseq_otu_krona sourcing krona/ktimporttext from nf-core#121

Merged
mberacochea merged 17 commits intomainfrom
update/mapseq_otu_krona
Jan 22, 2026
Merged

mapseq_otu_krona sourcing krona/ktimporttext from nf-core#121
mberacochea merged 17 commits intomainfrom
update/mapseq_otu_krona

Conversation

@vagkaratzas
Copy link
Contributor

No description provided.

@vagkaratzas vagkaratzas requested a review from chrisAta October 21, 2025 13:37
Copy link
Member

@chrisAta chrisAta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but having a further look at the MAPseq module I remember that the developers released a patch to the newest version that makes the output deterministic. The amplicon pipeline already uses that version, see here: https://github.com/EBI-Metagenomics/amplicon-pipeline/blob/f189b2619c47fbb42655cda25b3cc7f9e08f6f14/modules/ebi-metagenomics/mapseq/main.nf#L6-L9

I think I forgot to change the conda container version but otherwise it should be mapseq:2.1.1b. Do you think you could update this too? That way this desync never happens again haha

@vagkaratzas
Copy link
Contributor Author

Looks good to me, but having a further look at the MAPseq module I remember that the developers released a patch to the newest version that makes the output deterministic. The amplicon pipeline already uses that version, see here: https://github.com/EBI-Metagenomics/amplicon-pipeline/blob/f189b2619c47fbb42655cda25b3cc7f9e08f6f14/modules/ebi-metagenomics/mapseq/main.nf#L6-L9

I think I forgot to change the conda container version but otherwise it should be mapseq:2.1.1b. Do you think you could update this too? That way this desync never happens again haha

On it. I'll do it in this PR..just changing the conda and docker containers

@vagkaratzas
Copy link
Contributor Author

@mberacochea @chrisAta the schema that is defined in the modules folder is different to the one that nf-core is checking with nf-core tools. So I cannot debug the error I am afraid. EIther we need to update the schema in the repo (probably all modules will complain afterwards though), or bypass the json schema linting, or you can fix if you know what's wrong :P

Copy link
Member

@chrisAta chrisAta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not done yet but noticed this real quick

@chrisAta
Copy link
Member

@mberacochea @chrisAta the schema that is defined in the modules folder is different to the one that nf-core is checking with nf-core tools. So I cannot debug the error I am afraid. EIther we need to update the schema in the repo (probably all modules will complain afterwards though), or bypass the json schema linting, or you can fix if you know what's wrong :P

I just tried and ended up in the same death loop you did 💀 not sure what we should do? @mberacochea

@mberacochea mberacochea force-pushed the update/mapseq_otu_krona branch from 7291927 to b43fbfb Compare January 22, 2026 09:50
@mberacochea mberacochea requested a review from chrisAta January 22, 2026 09:56
Copy link
Member

@mberacochea mberacochea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit: .. unless @chrisAta has any objections

Copy link
Member

@chrisAta chrisAta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg so happy to see this fixed now, thanks @mberacochea !!

@mberacochea mberacochea merged commit b87bce8 into main Jan 22, 2026
7 checks passed
@mberacochea mberacochea deleted the update/mapseq_otu_krona branch January 22, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants