Skip to content

Remove support for legacy sample sheet and adjust multiregion sheet#1028

Merged
d4straub merged 3 commits into
nf-core:devfrom
d4straub:remove-support-for-legacy-datasheet
Jul 24, 2026
Merged

Remove support for legacy sample sheet and adjust multiregion sheet#1028
d4straub merged 3 commits into
nf-core:devfrom
d4straub:remove-support-for-legacy-datasheet

Conversation

@d4straub

@d4straub d4straub commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

This depends on nf-core/test-datasets#2164.
Because we do anyway the next major version, we can also slim & adapt parsed input files and change requirement (to the better).
This is essentially reverting #981 to have only one standardized sample sheet available for the pipeline, making maintenance simpler.
Additionally, multi-region sheet is adapted to streamline with parameter changes in #1027, specifically renaming FW_primer to primer_fwd and RV_primer to primer_rev.

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/ampliseq 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).

@d4straub
d4straub marked this pull request as ready for review July 23, 2026 10:58

@erikrikarddaniel erikrikarddaniel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

I have converted "my" pipelines to initialize all channels from parameters in the PIPELINE_INITIALISATION function and let the main workflow take a list of arguments corresponding to those channels plus any value params, thus avoiding params.* in the code. I suppose the main advantage for this is if one wants to call the workflow from another pipeline or so, i.e. not only the root directory main.nf. But it's also cleaner, IMO, not to have params.* scattered deep into the file structure. I think Claude could probably be quite helpful with this type of task. This would of course be a different PR than this, but maybe a good time now that it's time for a major bump? If you agree, I could do this once you have merged this PR.

@d4straub

Copy link
Copy Markdown
Collaborator Author

Looks good.

Thanks!

I have converted "my" pipelines to initialize all channels from parameters in the PIPELINE_INITIALISATION function and let the main workflow take a list of arguments corresponding to those channels plus any value params, thus avoiding params.* in the code. I suppose the main advantage for this is if one wants to call the workflow from another pipeline or so, i.e. not only the root directory main.nf. But it's also cleaner, IMO, not to have params.* scattered deep into the file structure. I think Claude could probably be quite helpful with this type of task. This would of course be a different PR than this, but maybe a good time now that it's time for a major bump? If you agree, I could do this once you have merged this PR.

Yes I have also thought about that. A disadvantage seems to me that another layer of complexity is introduced into the code by the necessity to forward params at multiple places (from entry level to deep into the file structure). It sort of makes it transparent what is required for each step but also clunkier, imho.
But I think its a good idea. The pipeline has a relatively high number of local modules though that occasionally might historically have params uncomfortably deep in the file tree. But it should be doable.

@d4straub
d4straub merged commit 4530155 into nf-core:dev Jul 24, 2026
76 of 81 checks passed
@d4straub
d4straub deleted the remove-support-for-legacy-datasheet branch July 24, 2026 06:49
@erikrikarddaniel

Copy link
Copy Markdown
Member

Looks good.

Thanks!

I have converted "my" pipelines to initialize all channels from parameters in the PIPELINE_INITIALISATION function and let the main workflow take a list of arguments corresponding to those channels plus any value params, thus avoiding params.* in the code. I suppose the main advantage for this is if one wants to call the workflow from another pipeline or so, i.e. not only the root directory main.nf. But it's also cleaner, IMO, not to have params.* scattered deep into the file structure. I think Claude could probably be quite helpful with this type of task. This would of course be a different PR than this, but maybe a good time now that it's time for a major bump? If you agree, I could do this once you have merged this PR.

Yes I have also thought about that. A disadvantage seems to me that another layer of complexity is introduced into the code by the necessity to forward params at multiple places (from entry level to deep into the file structure). It sort of makes it transparent what is required for each step but also clunkier, imho. But I think its a good idea. The pipeline has a relatively high number of local modules though that occasionally might historically have params uncomfortably deep in the file tree. But it should be doable.

I'm using Claude for a lot of coding now, and this type of task is perfect. Should I give it a go?

@d4straub

Copy link
Copy Markdown
Collaborator Author

I'm using Claude for a lot of coding now, and this type of task is perfect. Should I give it a go?

Yes!

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.

2 participants