Skip to content

Fix linting and local sbwf#293

Open
LouisLeNezet wants to merge 13 commits into
nf-core:devfrom
LouisLeNezet:fix_linting_sbwf
Open

Fix linting and local sbwf#293
LouisLeNezet wants to merge 13 commits into
nf-core:devfrom
LouisLeNezet:fix_linting_sbwf

Conversation

@LouisLeNezet
Copy link
Copy Markdown
Collaborator

PR checklist

In this PR, I fixed some small issues present in local subworkflows.
I've updated the documentation and added new tests.

  • 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/phaseimpute 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).

@LouisLeNezet LouisLeNezet self-assigned this May 21, 2026
Copy link
Copy Markdown
Collaborator

@atrigila atrigila left a comment

Choose a reason for hiding this comment

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

Mostly good, just one question

// Prepare reference genome files
//

include { SAMTOOLS_FAIDX } from '../../../modules/nf-core/samtools/faidx'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I agree that it doesn't check the nf-core guidelines as a sbwf.
However, I would argue that these guidelines are meant for the nf-core/modules repository, to avoid bloating the repo with trivial sbwf. I think that for local sbwf this constraint is maybe not relevant, as this allow to properly isolate and test channel handling while reducing the length of huge main.nf file.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't understand the decision of having a prepare_genome subworkflow. What was the issue before with how things were done? Thanks.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was a recommendation from Maxime to take out the genome indexing out of the local phaseimpute utils sbwf.
I agree that the sbwf is a bit trivial, but the "complex" channel handling make it fit from my point of view as a local sbwf.

@LouisLeNezet
Copy link
Copy Markdown
Collaborator Author

Hi @atrigila,
I've slightly update how the panel_id are obtained to fix an issue when no panel/posfile/chunks are provided (e.g. only simulate step).
I've added the related test.
Is it good for you ?

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