modules icon indicating copy to clipboard operation
modules copied to clipboard

Parameterising args for RTGTOOLS_FORMAT

Open steven-rusk opened this issue 9 months ago • 6 comments

PR checklist

Closes #XXX

  • [ ] 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 module conventions in the contribution docs
  • [ ] If necessary, include test data in your PR.
  • [ ] Remove all TODO statements.
  • [ ] Emit the versions.yml file.
  • [ ] Follow the naming conventions.
  • [ ] Follow the parameters requirements.
  • [ ] Follow the input/output options guidelines.
  • [ ] Add a resource label
  • [ ] Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • [ ] nf-core modules test <MODULE> --profile docker
      • [ ] nf-core modules test <MODULE> --profile singularity
      • [ ] nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • [ ] nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • [ ] nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • [ ] nf-core subworkflows test <SUBWORKFLOW> --profile conda

steven-rusk avatar Mar 24 '25 03:03 steven-rusk

Hi, What is the reason for this PR? Modules should generally just use task.ext.args to pass optional arguments directly to the tool.

SPPearce avatar Mar 24 '25 06:03 SPPearce

Hi, What is the reason for this PR? Modules should generally just use task.ext.args to pass optional arguments directly to the tool.

@SPPearce The reason for this PR is to allow for some type-checking and input validation for those optional parameters. $task.ext.args seems to just be a string - do you know if there is an established pattern for input validation of this single string variable?

steven-rusk avatar Mar 24 '25 07:03 steven-rusk

Yes, task.ext.args is typically just a string (there is one module I can think of which passes it as a list, but that is non-standard). The type checking is normally just left to the tool itself to complain about. What do you mean by "input validation"? Anything that requires a file needs to be treated separately as an input channel.

SPPearce avatar Mar 24 '25 07:03 SPPearce

Yes, task.ext.args is typically just a string (there is one module I can think of which passes it as a list, but that is non-standard). The type checking is normally just left to the tool itself to complain about. What do you mean by "input validation"? Anything that requires a file needs to be treated separately as an input channel.

@SPPearce Thanks for clarifying. Several of the possible optional args for this module do require files, so good to know that those should always be handled as inputs - I can at least make that change. If leaving the type-checking for non-file optional args up to the tool is the established norm, then that's what I'll do. Thanks for your feedback (and I'll be sure to have a proper read through that module specification doc you linked earlier).

steven-rusk avatar Mar 24 '25 07:03 steven-rusk

Ok, thanks. Yes, anything that can be a file input must be staged via the input channels, as Nextflow expects all its inputs to be inside the work directory. On a HPC or locally you can get away with pointing to a random file in your system (although bad practice), but when executing on a cloud infrastructure that doesn't work and only the files that are being input will actually be present.

SPPearce avatar Mar 24 '25 07:03 SPPearce

Also I note that one of the arguments is called seed. If that is controlling randomness, does the default produce reproducible results? If not, then I'd be keen for that one to be checked for. For example or here

SPPearce avatar Mar 24 '25 07:03 SPPearce