add MetaBinner

[d4straub]

Add MetaBinner, related to #874
Needs #907 to function properly.

This integrates into the pipeline an altered version of the module METABINNER and the script create_metabinner_bins.py from https://github.com/hzi-bifo/mag.

Issues:

• MetaBinner fails with -profile test_full to produce any bins (but that is error is ignored)
• running on bacass test_full data works as detailed below
• doesnt produce any bins with any CI test datasets (all test profiles with --skip_comebin false --skip_metabinner false failed with those binners) therefore deactivated in all CI tests

Successful test was with bacass test_full files:

wget  ftp://ftp.sra.ebi.ac.uk/vol1/fastq/SRR100/028/SRR10093028/SRR10093028_1.fastq.gz
wget  ftp://ftp.sra.ebi.ac.uk/vol1/fastq/SRR100/029/SRR10093029/SRR10093029_1.fastq.gz
wget  ftp://ftp.sra.ebi.ac.uk/vol1/fastq/SRR100/028/SRR10093028/SRR10093028_2.fastq.gz
wget  ftp://ftp.sra.ebi.ac.uk/vol1/fastq/SRR100/029/SRR10093029/SRR10093029_2.fastq.gz
nextflow run d4straub/mag -r add-MetaBinner -profile cfc --input samplesheet_bacass_test.csv --outdir results_test_bacass -resume --skip_spades --skip_quast --skip_busco --skip_prodigal --skip_prokka --skip_gtdbtk --refine_bins_dastool --keep_phix --coassemble_group --skip_comebin

where the samplesheet contained:

sample,group,short_reads_1,short_reads_2,long_reads,short_reads_platform,long_reads_platform
SRR10093028,a,./SRR10093028_1.fastq.gz,./SRR10093028_2.fastq.gz,,ILLUMINA,
SRR10093029,a,./SRR10093029_1.fastq.gz,./SRR10093029_2.fastq.gz,,ILLUMINA,

PR checklist

• This comment contains a description of changes (with reason).
• If you've fixed a bug or added code that should be tested, add tests!
• If you've added a new tool - have you followed the pipeline conventions in the contribution docs
• If necessary, also make a PR on the nf-core/mag branch on the nf-core/test-datasets repository.
• Make sure your code lints (nf-core pipelines lint).
• Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
• Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
• Usage Documentation in docs/usage.md is updated.
• Output Documentation in docs/output.md is updated.
• CHANGELOG.md is updated.
• README.md is updated (including new tool citations and authors/contributors).

[nf-core-bot]

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.3.2.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

[d4straub]

@nf-core-bot fix linting

[d4straub]

Now ready for review, will resolve that conflict with ro-crate -metadata.json soon, but that wont impact the functional code at all.

[jfy133]

I agree with @prototaxites that the module is too complex at the moment.

Most of the steps should be in a CONCOCT-like workflow, and the the two python scripts and metabinnner itself should definitely be modules.

The other reason I would prefer to split this up is because it will make it easier to debug.

I'm not too happy about having to have custom commands and scripts (not your fault, just design of the tool) and I want to decouple these potentially more brittle parts from the tool itself.

[d4straub]

Alright, split the module into 4 modules and added a subworkflow.
Tested as in opening post & works.
Due to the ongoing release prep for 5.1.0 I am waiting until after the release for merging usptream dev.

[d4straub]

Ready for another round of reviews, maybe @jfy133 & @prototaxites again?

[dialvarezs]

Minor suggestions from my side.
With the subworkflow implementation is definitively easier to follow.

[d4straub]

@nf-core-bot fix linting

[d4straub]

I'll run the pipeline as usual before merging this PR to confirm I didnt break anything, but our cluster is a little over-used by me right now so it takes some time.
Edit: Success!

[prototaxites]

This is looking good @d4straub, a few small comments from me

[prototaxites]

Should these be ext directives, or (if this is mandatory) should it just be an input to the module?

[d4straub]

Those arent mandatory, since they have defaults.

[prototaxites]

But it would be mandatory in the sense that if there was no default, the tool wouldn't run?

[d4straub]

Yes, the tool needs that min_contig_size. The process has default 1000 in case there is no ext.min_contig_size.

[prototaxites]

In which case I would suggest for clarity that it should be an optional val input to the module, and you set a default in the same way. That means that all of the mandatory inputs for the module are set in the same place.

Following the logic from Mahesh here: nf-core/proposals#69

[d4straub]

Done!

[prototaxites]

I think this is fine for now, but if we consider in future to put this module in the modules repo, we should move this elsewhere to make the module more reusable

[prototaxites]

LGTM (assuming tests pass!) - but we definitely need to work on a suitable test data set!

[d4straub]

Yes, suitable datasets would be great. Its not only for testing itself, but also implementing tools that do not run on current test data is troublesome.
Edit: I make sure I get it to run reproducible as detailed in the opening post. But its certainly not optimal.

[d4straub]

Test Run nf-test / docker | 25.04.2 | 2/8 (pull_request) = '-profile test_alternatives' fails with

! 85         "busco_summary.tsv:md5,f21 ! 85         "busco_summary.tsv:md5,7a3
08b2ad319f7bdbe1d42c185e34e43",         afa3387a815b331c35a3420d8c7f4",    

I'll retry, potentially its a sorting issue.

Edit: no, that seems to be consistent, investigating...

[jfy133]

@dialvarezs you also saw this in the verison bump PR right?

[dialvarezs]

@jfy133 Yes, the same. #912 should fix it.

[d4straub]

Ah great than I dont need to investigate :)
So I wait until #912 is merged?

[dialvarezs]

It's ready now @d4straub!

[d4straub]

Thanks everybody!!!!