allow mixed str and dict in lint config #3228
Conversation
|
Could you write a quick test for this as well? |
…ix-yml-config-type
There was a problem hiding this comment.
Tried my best but am not entirely following with all changes due to a lack of knowledge on the codebase of nf-tools.
Tested it also again on my pipeline & new failed tests were popping up because actions_ci was not included in the class NFCoreYamlLintConfig(BaseModel
| assert len(results.get("failed", [])) == 0 | ||
| assert len(results.get("warned", [])) == 0 | ||
| assert len(results.get("ignored", [])) == 0 |
There was a problem hiding this comment.
Why can these be all equal to 0? I would assume that we would have some ignored?
There was a problem hiding this comment.
we only lint the .nf-core.yml file here, so the above settings don't ignore anything there.
nf_core/utils.py
Outdated
| files_unchanged: Union[bool, List[str]] = [] | ||
| """ List of files that should not be changed """ | ||
| modules_config: Optional[Union[bool, List[str]]] = [] | ||
| """ List of modules that should not be changed """ | ||
| merge_markers: Optional[Union[bool, List[str]]] = [] | ||
| """ List of files that should not contain merge markers """ | ||
| nextflow_config: Optional[Union[bool, List[Union[str, Dict[str, List[str]]]]]] = [] | ||
| """ List of Nextflow config files that should not be changed """ | ||
| multiqc_config: Union[bool, List[str]] = [] | ||
| """ List of MultiQC config options that be changed """ | ||
| files_exist: Union[bool, List[str]] = [] | ||
| """ List of files that can not exist """ |
There was a problem hiding this comment.
Why not all optional?
For instance, I can imagine. multiqc_config being absent
There was a problem hiding this comment.
Optional here, means that it can be of the given type or None and because we give them a default value != None, there is no need for that.
| """ List of files that can contain template strings """ | ||
| nfcore_components: Optional[bool] = None | ||
| """ Include all required files to use nf-core modules and subworkflows """ | ||
|
|
There was a problem hiding this comment.
Would double check docs for some that you have missed.
- actions_ci
- actions_aws*
- ...
Or are they not necessary?
https://nf-co.re/tools/docs/latest/pipeline_lint_tests/
we should probably rewrite this into a TypedDict or similar in the future.