modules
modules copied to clipboard
Batch module updates - Summer 2024
[!WARNING] Please open PRs against the
batch_update_stagingbranch. Then we can have small, easy to review, atomic PRs but also merge all batch updates to the modules repo in one go.
[!NOTE] [Edit - @ewels]: I've split into two groups, those that will require changes from pipeline developers (other than just pulling in updated modules as normal) and those that will not (those that will require changes to pipeline code).
Anything that doesn't need changes to pipeline code can be done iteratively if we wish, rather than batched.
### ✅ Will not require pipeline updates ✅ ✅ ✅ ✅ ✅
- [ ] https://github.com/nf-core/modules/issues/4983
- [ ] https://github.com/nf-core/modules/issues/5830
- [ ] https://github.com/nf-core/modules/issues/5831
- [ ] https://github.com/nf-core/modules/issues/5829
- [ ] https://github.com/nf-core/modules/issues/5835
- [ ] https://github.com/nf-core/modules/issues/5836
- [ ] https://github.com/nf-core/modules/issues/5848
- [ ] https://github.com/nf-core/modules/issues/4570
- [ ] https://github.com/nf-core/modules/issues/6001
- [ ] https://github.com/nf-core/modules/issues/6454
- [ ] https://github.com/nf-core/modules/issues/6226
- [ ] https://github.com/nf-core/modules/issues/6504
- [ ] https://github.com/nf-core/modules/issues/6505
- [ ] https://github.com/nf-core/modules/issues/6650
### ⚡ Will require pipeline updates ⚡⚡⚡⚡⚡⚡⚡⚡
- [ ] https://github.com/nf-core/modules/issues/4517
- [ ] https://github.com/nf-core/modules/issues/5834
- [ ] https://github.com/nf-core/modules/issues/5832
- [ ] https://github.com/nf-core/modules/issues/5833
Just a heads up for bulk changes: there is a strict matrix limit of 256 tests in a test strategy on github, so that is the max number of module changes in a PR. @mashehu #2738
Can we automatically swap the test datasets paths too?
@SPPearce can you elaborate / open an issue about this please? (Or point to an existing one)
@SPPearce can you elaborate / open an issue about this please? (Or point to an existing one)
We have swapped the file paths used in tests from:
file(params.test_data['sarscov2']['genome']['genome_fasta'], checkIfExists: true)
to direct paths like:
file(params.modules_testdata_base_path + 'genomics/sarscov2/genome/genome.fasta', checkIfExists: true)
This is in the guidelines now, but could do with a bulk update.
Made an issue: https://github.com/nf-core/modules/issues/5848
My concern is that several of the changes require Nextflow 24.04. It means that from this point forward, nf-core will only support Nextflow 24.04+. Is there an analysis of 24.04 vs 22.04-23.10 and what developers have to do to make their pipelines compatible with 24.04 ? Any pipeline developer who is unable to move to 24.04 would remain stuck on pre-June 2024 modules and sub-workflows.
@muffato this has been policy for a long time now in nf-core. We are not adopting anything that requires an edge release of Nextflow here, everything is stable. There are other updates that are in 24.04 that we also want to update for pipelines, such as the use of resourceLimits. Some of those changes have been requested for ~6 years, and have been present in the Nextflow edge release for ~6 months.
We will soon bump the minimum required version of Nextflow in the pipeline template, and many / most of these changes should come through automatically via template syncs and module updates. We will do our best to write lint tests to cover most of them so that any required changes don't slip through the gaps. We've also recently started the practice of writing blog posts and doing bytesize talks where we talk over and demonstrate the updates that are required for new versions of the template.
I appreciate that it can be frustrating to keep updating pipelines with these kinds of changes. But that's also part of what makes nf-core great. We are a large pool of feature requests to Nextflow which makes it better. Stability is still possible through using previous versions of Nextflow and pipelines. I don't think that we should be held back from implementing new features once they are available in stable versions of Nextflow.
Any pipeline developer who is unable to move to 24.04
The hope is that no-one falls into this camp. If you can see any potential reasons for this, then please say and we can try to resolve them.
Oh yes @ewels , I don't deny the advantages of moving over. There will be a lot of benefits for people to move. Hopefully we can get away with it by just updating the Nextflow version number and can do pipeline template updates later.
Should this also include adding stubs? (That is not a breaking change but might still be worth to keep in mind?) Related to #4570.
Should this also include adding stubs? (That is not a breaking change but might still be worth to keep in mind?) Related to #4570.
Added, thanks!
Can we add, including arity: to modules or is that out of scope? It'll require a more recent minimum version.
@mahesh-panchal - we will be requiring a pretty recent minimum version of Nextflow for a lot of this (see comments above), so I think that should be fine 👍🏻
Do we have an issue for that? Could you write one if not? Then we can add it to the list above. Thanks!
There's not an issue, but a PR #5915. I'll make an issue.
Added https://github.com/nf-core/modules/issues/6454 to list
I also added pytest port #6226 . I have been experimenting with some automation. But there is still quite a bit of work remaining, round 170 single process pytests and around 200 multi-process pytests. @SPPearce has been very helpful in reviewing the PRs.
Happy to try tackling some of the issues here. Is the plan to tackle this module by module?, or try to script some of the changes, before bulk checking that the modules still function.
Happy to try tackling some of the issues here. Is the plan to tackle this module by module?, or try to script some of the changes, before bulk checking that the modules still function.
They need scripted changes, not manual updates per file. It is extremely painful to do anything manually across all ~1000 modules :|
Closing this in favor of #8239 and #8238 :)