Skip to content

Conversation

@matrulda
Copy link
Contributor

@matrulda matrulda commented Jan 19, 2026

Fixes #166. The RUNDIRPARSER module is now skipped if there is no rundir information.

With this in place, I see no need to further document the rundir. It currently says it is optional and now it can be left empty without the pipeline breaking.

We probably should add a test for this (basically have a sample sheet with no rundir information), I have not done this (yet).

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

@matrulda matrulda marked this pull request as draft January 19, 2026 12:55
@matrulda matrulda self-assigned this Jan 19, 2026
@matrulda
Copy link
Contributor Author

nf-core/test-datasets#1836 Needs to be merged before I can add a nf-test.

// From samplesheet channel serving (sampleMetaObj, sampleReadsPath) tuples:
// --> Create new rundir channel serving (rundirMetaObj, rundirPath) tuples
ch_rundir = ch_samplesheet
.filter { meta, _reads -> meta.rundir.size() > 0 }
Copy link
Member

Choose a reason for hiding this comment

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

why not printing the warning here so that we know for which sample we have no rundir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I had it like that first, but it results in a lot of warning if there are a lot of samples. Since some users might run this pipeline without ever providing any rundir information, I thought it might be cleaner to just report if the rundirparser step is skipped completely.

@matrulda matrulda marked this pull request as ready for review January 19, 2026 15:56
@matrulda
Copy link
Contributor Author

@beatrizsavinhas Since you reported the issue it would be nice to have your input as well. Is the automatic skip enough or do we need more information about the run directory? 🙂

@beatrizsavinhas
Copy link
Contributor

I think this is great! 🌟
In terms of usability I think this is good enough, but maybe it would be useful to have some written information somewhere about this step? Just so the user could know what it does and when it runs or not.
Perhaps something in the docs/usage.md file, in the table that lists the columns of the samplesheet, like this?

rundir Path to the runfolder containing extra information about the sequencing run (optional*).

(...)

*If rundir is not provided, the RUNDIRPARSER module is skipped and the run metadata is not collected.

@matrulda
Copy link
Contributor Author

I think this is great! 🌟 In terms of usability I think this is good enough, but maybe it would be useful to have some written information somewhere about this step? Just so the user could know what it does and when it runs or not. Perhaps something in the docs/usage.md file, in the table that lists the columns of the samplesheet, like this?

rundir
Path to the runfolder containing extra information about the sequencing run (optional*).

(...)
*If rundir is not provided, the RUNDIRPARSER module is skipped and the run metadata is not collected.

Sounds great, will fix! Thank you! 🙏

@matrulda matrulda requested a review from maxulysse January 23, 2026 06:26
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.

documentation for run folder inclusion

3 participants