Skip to content

Only print default parameter values that exist in schema - #1

Open
d-goryslavets wants to merge 4 commits into
sanger-pathogens:mainfrom
d-goryslavets:make-default-value-optional
Open

Only print default parameter values that exist in schema#1
d-goryslavets wants to merge 4 commits into
sanger-pathogens:mainfrom
d-goryslavets:make-default-value-optional

Conversation

@d-goryslavets

Copy link
Copy Markdown

Description

If a workflow parameter is mandatory, the help message should not display a default value for it. Currently, if the default field is omitted for a parameter in the schema, the help message displays it as null. For instance, for this schema:

{
  "pipeline": "Base schema",
  "params": {
    "Pipeline options": {
      "indir": {
        "help_text": "Directory with isolates to annotate"
      }
    }
  }
}

The help message is:

Pipeline options 
    --indir
          default: null
          Directory with isolates to annotate

which is somewhat misleading. I suggest not printing default: null for mandatory parameters.

Changes

I made printing the default field in the help_message method conditional on the presence of the default key.

@d-goryslavets

Copy link
Copy Markdown
Author

I've added a check for an extra boolean field required in the schema for each parameter. It is treated as mutually exclusive with the default property, meaning that if a default value is set, the parameter cannot be required (i.e. mandatory for the user to provide). So, using the example schema above:

{
  "pipeline": "Base schema",
  "params": {
    "Pipeline options": {
      "indir": {
        "required": true,
        "help_text": "Directory with isolates to annotate"
      }
    }
  }
}

We get the following help message:

 Pipeline options 
      --indir
            <required>
            Directory with isolates to annotate

Note that I kept the default field optional. This is because a parameter can be optional while its default value is meaningless. For instance, consider this schema:

{
  "pipeline": "Base schema",
  "params": {
    "Pipeline options": {
      "indir": {
        "required": true,
        "help_text": "Directory with isolates to annotate"
      }
    },
    "Annotation option": {
      "user_proteins": {
        "help_text": "User-provided expert proteins in FASTA format for CDS annotation"
      },
      "user_hmms": {
        "help_text": "User-provided hidden markov model in HMMER format for CDS annotation"
      }
    }
  }
}

The user_proteins and user_hmms parameters are not mandatory, but their default value is null, which I don't think should be printed (please let me know if you think otherwise). Thus, the help message for that schema is:

 Pipeline options 
      --indir
            <required>
            Directory with isolates to annotate
-----------------------------------------------------------------
 Annotation option 
      --user_proteins
            User-provided expert proteins in FASTA format for CDS annotation
      
      --user_hmms
            User-provided hidden markov model in HMMER format for CDS annotation      
-----------------------------------------------------------------

@flass

flass commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

thank you Dima for the suggestion, it makes a lot of sense like this. I agree that when the default is a null (Groovy value), it is better not to display it as such as it can be misleading, as for instance trying to privide the 'null' string a argument value through the CLI will result into a string being ingested, not the intended null value obtained by defining it in the config.
The default behaviour of the pipeline however should be described in the help message, as usually it would not be explicit what a null value for an argument entails.

@flass flass left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this might be a bit pedantic but I think introducing the suggested function would help not inflate this piece of groovy code that is already quite large and hard to digest

Comment thread NextflowTool.groovy Outdated
@flass

flass commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Ok so I've tested the latest commit in irods_Extractor, just printing the help message, and it seems to break it - not sure where though. the help message seems to be printed in full, but it hits something where something seems now to refer to a variable called default that's not defined

[fl4@farm22-head2 lib]$ git remote add --fetch d-goryslavets/nextflowtool https://github.com/d-goryslavets/nextflowtool
Updating d-goryslavets/nextflowtool
remote: Enumerating objects: 11, done.
remote: Counting objects: 100% (11/11), done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 9 (delta 3), reused 9 (delta 3), pack-reused 0 (from 0)
Unpacking objects: 100% (9/9), 1.51 KiB | 70.00 KiB/s, done.
From https://github.com/d-goryslavets/nextflowtool
 * [new branch]      main                        -> d-goryslavets/nextflowtool/main
 * [new branch]      make-default-value-optional -> d-goryslavets/nextflowtool/make-default-value-optional
[fl4@farm22-head2 lib]$ git checkout d-goryslavets/nextflowtool/make-default-value-optional
Previous HEAD position was 9fbcf3c Merge branch 'cli_parse_update' into 'main'
HEAD is now at 427be64 refactor: move argument block printing to a separate method
[fl4@farm22-head2 lib]$ cd ~/lustre/test_irods_extractor/
[fl4@farm22-head2 test_irods_extractor]$ module load nextflow/25.10.3 ISG/singularity/
        Module loaded. For more information run 'module help nextflow/25.10.3'.

Loading nextflow/25.10.3
  Loading requirement: openjdk-17.0.8.1_1
[fl4@farm22-head2 test_irods_extractor]$ nextflow run ../software/sanger-pathogens/pipelines/irods_extractor/main.nf --help
[...]
      --track_batch_metadata
            default: false
            Track batch-level metadata files for the pipeline run (pipeline run manifest, reads QC summary, trimming and host read removal stats summaries).
-----------------------------------------------------------------
 only_new_input
      --type
-[Irods-extractor] Pipeline errored before completion-
Summary statistics:
- Successfully completed processes: 0
- Failed and retried processes: 0
- Failed and ignored processes: 0
- Total processes: 0
ERROR ~ No such variable: default

@flass flass left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test made on my side failed. I suspect there is an unbound variable somewhere

Comment thread NextflowTool.groovy Outdated
// print argument key, help text and type (default/required), if provided
public static void printHelpMessageBlock(argumentKey, argumentBlock, indent, log) {
log.info indent + "--" + argumentKey
if (argumentBlock.default) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe this does not work when there is no attribute that's been defined with this name for the block

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @flass! Thanks for testing this on your end. While trying to reproduce this, I noticed that this error pops up even without my changes (that is, using the plain irods_extractor workflow). After some digging I'm confident that the problem lies in the incorrect schema structure in one of the irods_extractor's dependencies.

Specifically, it's the pipeline_chaining schema that causes the error. That schema lacks an outer named block for the parameters, which is expected by the NextflowTool parser.

I've opened a merge request on GitLab fixing this. Please check if out when you have time.

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