-
Notifications
You must be signed in to change notification settings - Fork 144
Json schema improvements #777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
assets/schema_input.json
Outdated
| "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!" |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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!
nextflow_schema.json
Outdated
| "type": "boolean", | ||
| "description": "Specifies that the input is single-end reads.", | ||
| "fa_icon": "fas fa-align-center", | ||
| "default": false, |
There was a problem hiding this comment.
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
nextflow_schema.json
Outdated
| }, | ||
| "bowtie2_mode": { | ||
| "type": "string", | ||
| "type": ["string", "boolean"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a boolean here?
There was a problem hiding this comment.
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 ?
nvnieuwk
left a comment
There was a problem hiding this 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 :)
Co-authored-by: Nicolas Vannieuwkerke <[email protected]>
Co-authored-by: Nicolas Vannieuwkerke <[email protected]>
|
I have encountered an unexpected behaviour when executing the following command: The above results in the following warning: This results in the json receiving a boolean instead of a string. The |
|
I believe the only correct way to supply a param value which starts with |
|
@nf-core-bot fix linting |
I checked with Arthur's solution and adapted the |
| "default": "https://raw.githubusercontent.com/nf-core/test-datasets/", | ||
| "hidden": true | ||
| "hidden": true, | ||
| "help_text": "Location of pipeline test dataset files" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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]*$", |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
|
Thanks @agusinac ! |
This PR updates the JSON schema to be more complete