parameter validation for dev branch

[KevinMenden]

New PR to merge parameter validation into dev instead of dsl2_template

[apeltzer]

Looks good to me - maybe two things:

• rename the lib file to something with nfcore in the filename?

• add in the other things that came with the libs from rnaseq while you are at it?

• Automatic help generation

• Automatic summary generation

• header generation

All of these have been added to eager already and work very well together with the validation and since we're now taking the effort to backporting this to DSLv1 template (and later to DSLv2 of course), would make sense to keep this already in line with the DSLv2 branch no?

[apeltzer]

And what Phil mentioned #817 (comment)

[KevinMenden]

Yes thought about that - might as well do it now 👍

Okay suggestions for the magic jar file are still up for grabs 😁

[ewels]

Got to Headers.groovy and submitting early for discussion..

[ewels]

Just pushed a commit 3cfeea3 to the PR that makes the error output a little friendlier..

Instead of:

Found parameter violations!
#: only 1 subschema matches out of 2
#: only 4 subschema matches out of 5
expected type: Boolean, found: String #/single_end

It now prints:

Error, validation of pipeline parameters failed!
- params.single_end: expected type: Boolean, found: String

[ewels]

Ok, made a few more changes:

• Nicer log messages for missing required params
• Show unexpected params if we get a validation error
• New params.schema_ignore_params to allow unexpected params that are not in the schema
• Show param value when there's a validation error with a specific param
• Set --input to null in nextflow.config as we can now catch missing required params before the channel is set up

Absolutely loving how well this works, going to be an absolute game changer ✨

[ewels]

Ah I broke linting with the new parameter. Will take a look ASAP - I think it probably makes sense to get the linting to handle this and ignore it (and any other params it mentions). And probably add some docs.

[ewels]

Some more improvements:

• Don't exit before we do all of the different parameter validation checks. Means you get all errors in one go instead of having to fix one (eg. core nextflow option) only to rerun and get a different one (schema validation error).
• Some more minor tweaks and tidying to the logging output
• The vast majority of core Nextflow options were missing (I noticed when I tried --resume and it didn't stop me). I took the list from the Nextflow source code and added basically all of them (only omitted ones that are prefixes, eg -process.<foo>).

Looking pretty fly! 🚀

[ewels]

Ok, final change that I can think of - just added params.schema_ignore_params to nf-core schema lint and also the pipeline validation code (warning of unexpected parameters). I changed it to be a comma-separated list of parameter names. These are allowed to be absent in the pipeline schema.

I'm hoping that this should fix the CI tests.

[KevinMenden]

Wow, awesome, thanks a lot!
I had the feeling there were some nextflow options missing when I read something about a list or info option last week 😁 tried to scrape them together in the doc.

Cool so all that's left to do is adding the remaining parameters to the schema, right? I'll do that now and then I think we're pretty much there unless you have some more ideas :)

[KevinMenden]

Okay saw that you put the genomes param in the ignore param list which works nice :)

Currently we are reporting unexpected params twice, directly when found and when the workflow fails.
Do you want to keep it like this or should we remove the code snippet in main.nf that reports them when failing?

[apeltzer]

Okay saw that you put the genomes param in the ignore param list which works nice :)

Currently we are reporting unexpected params twice, directly when found and when the workflow fails.
Do you want to keep it like this or should we remove the code snippet in main.nf that reports them when failing?

Sounds good to keep both I think. Seeing it from a user perspective, its very useful to get some feedback on failure, e.g. "these parameters have been set but shouldn't be" - especially good for beginners, as they might then start thinking about this and/or provide this when asking for help :-)

[apeltzer]

As nobody did it on paolos request on twitter, I thought maybe start the discussion over there: nextflow-io/nextflow#1893

Didn't want to win your laurels on that one, instead just wanted to make sure there is a follow up and we don't forget about it / delay it any further 👍🏻

[KevinMenden]

Awesome thanks @apeltzer !

[ewels]

My review is now a little pointless and I have pushed several commits to this PR now so am kind of a co-author. But I'm happy with this, I think it's good to be merged 👍🏻

[apeltzer]

I have tested all of the functionality in the linked PR on eager and things worked fine for me 🎉 happy for this to get merged :-)