Skip to content

Conversation

@agusinac
Copy link
Contributor

@agusinac agusinac commented Mar 24, 2025

This PR updates the JSON schema to be more complete

"exists": true,
"pattern": "^\\S+\\.f(ast)?q\\.gz$"
"pattern": "^\\S+\\.f(ast)?q\\.gz$",
"errorMessage": "short_reads_1 needs to be a file path with no spaces!"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also mention that the file needs to exist? (And also do this for each other file error message?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes I will do that!

"type": "boolean",
"description": "Specifies that the input is single-end reads.",
"fa_icon": "fas fa-align-center",
"default": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

The default false option isn't necessary for booleans as we assume a boolean to be false by default

},
"bowtie2_mode": {
"type": "string",
"type": ["string", "boolean"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a boolean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because when I tested it with "--very-sensitive" it returns a json file in the result with bowtie2_mode: true so it seems that the flag returns a boolean ?

Copy link
Contributor

@nvnieuwk nvnieuwk left a comment

Choose a reason for hiding this comment

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

Great work!!!! Here are some comments :)

agusinac and others added 3 commits March 24, 2025 15:54
@agusinac
Copy link
Contributor Author

agusinac commented Mar 24, 2025

I have encountered an unexpected behaviour when executing the following command:
nextflow run main.nf -profile test, docker --outdir results -stub --bowtie2_mode "--very-sensitive".

The above results in the following warning:
* --verySensitive: true

This results in the json receiving a boolean instead of a string. The nextflow_schema.json, line 743 has been changed to account for the boolean. After discussing with @nvnieuwk we thought this should be brought to your attention @jfy133

"bowtie2_mode": {
    "type": ["string", "boolean"],
    "description": "Bowtie2 alignment mode",
    "help_text": "Bowtie2 alignment mode options, for example: `--very-fast` , `--very-sensitive-local -N 1` , ... Must be used like this: --bowtie2_mode \"--very-sensitive\"",
    "pattern": "^\\'.*\\'$"
}

@awgymer
Copy link

awgymer commented Mar 24, 2025

I believe the only correct way to supply a param value which starts with -- is to use explicit equals:

 --bowtie2_mode="--very-sensitive"

@jen-reeve jen-reeve linked an issue Mar 25, 2025 that may be closed by this pull request
15 tasks
@jfy133
Copy link
Member

jfy133 commented Mar 25, 2025

Yeah I was personally never particularly happy with these flags, but it is what it is. Thanks for giving the solution @awgymer ! @agusinac could you check with Arthur's solution?

@jfy133
Copy link
Member

jfy133 commented Mar 25, 2025

@nf-core-bot fix linting

@agusinac
Copy link
Contributor Author

agusinac commented Mar 26, 2025

Yeah I was personally never particularly happy with these flags, but it is what it is. Thanks for giving the solution @awgymer ! @agusinac could you check with Arthur's solution?

I checked with Arthur's solution and adapted the nextflow_schema.json. The json file now returns a string, the help_text displays the correct way to call the flag as --bowtie2_mode="--very-sensitive" and json keyword pattern is updated to check the validation of the passed string.

"default": "https://raw.githubusercontent.com/nf-core/test-datasets/",
"hidden": true
"hidden": true,
"help_text": "Location of pipeline test dataset files"
Copy link
Member

Choose a reason for hiding this comment

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

Everything in this file above this comment are all from the template, have you updated it there too @agusinac (or more likely @nvnieuwk )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I went through the whole document and added keywords if they were missing.

"adapterremoval_adapter1": {
"type": "string",
"default": "AGATCGGAAGAGCACACGTCTGAACTCCAGTCACNNNNNNATCTCGTATGCCGTCTTCTGCTTG",
"pattern": "^[ATGCRYKMSWBDHVN]*$",
Copy link
Member

Choose a reason for hiding this comment

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

I have a feeling this would only accept AGCTN, but I can't really tell from the documentation... so we will stick with this for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should accept any of the mentioned characters to be present either zero or more times. The adapter2 didn't contain any N and passed the test. I am happy to hear if this should be improved in the future!

@jfy133
Copy link
Member

jfy133 commented Mar 26, 2025

Thanks @agusinac !

@jfy133 jfy133 merged commit 5e57b2e into nf-core:dev Mar 26, 2025
5 of 15 checks passed
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.

JSON schema: Mag

5 participants