nf-core / tools

Python package with helper tools for the nf-core community.
https://nf-co.re
MIT License
242 stars 191 forks source link

Add an option to make the new workflow name checks optional #2117

Closed nvnieuwk closed 1 year ago

nvnieuwk commented 1 year ago

Description of feature

When running nf-core lint on my pipeline it immediately fails with following error:

$ nf-core lint

                                          ,--./,-.
          ___     __   __   __   ___     /,-._.--~\
    |\ | |__  __ /  ` /  \ |__) |__         }  {
    | \| |       \__, \__/ |  \ |___     \`-._,-`-,
                                          `._,._,'

    nf-core/tools version 2.7.1 - https://nf-co.re

INFO     Testing pipeline: .                                                                                                                                         
ERROR    Invalid workflow name: must be lowercase without punctuation. 

The pipeline name is something we agreed on for all our pipeline so I can't simply change it. So an option to disable this check would be helpful. Thanks in advance!!

awgymer commented 1 year ago

I thought we added code to skip this:

https://github.com/nf-core/tools/blob/eecf1c7079ef89e918a7493fb2228ac2fc63e5e0/nf_core/create.py#L170-L180

In theory if you have an .nf-core.yml with manifest.name specified to be skipped then this won't get triggered? BUT I suppose we didn't think this through because how will you have one in a pipeline you are creating from scratch?

nvnieuwk commented 1 year ago

The .nf-core.yml config for the pipeline the command was run on looks like this:

repository_type: pipeline
lint:
  files_exist:
    - assets/nf-core-nfcmggstructural_logo_light.png
    - docs/images/nf-core-nfcmggstructural_logo_light.png
    - docs/images/nf-core-nfcmggstructural_logo_dark.png
    - .github/ISSUE_TEMPLATE/config.yml
    - .github/workflows/awstest.yml
    - .github/workflows/awsfulltest.yml
  nextflow_config:
    - manifest.name
    - manifest.homePage
  multiqc_config:
    - report_comment
  readme:
    - nextflow_badge
  files_unchanged:
    - .github/CONTRIBUTING.md
    - .github/ISSUE_TEMPLATE/bug_report.yml
    - .github/ISSUE_TEMPLATE/feature_request.yml
    - .github/PULL_REQUEST_TEMPLATE.md
    - .github/workflows/linting.yml
    - assets/email_template.txt
    - assets/sendmail_template.txt
    - lib/NfcoreTemplate.groovy
    - .prettierignore
  actions_ci: false

Am I missing something?

awgymer commented 1 year ago

I guess it may not be using that file? Could you try running with verbose/debug logging? It should tell you which yml file its using

nvnieuwk commented 1 year ago
$ nf-core --verbose lint

                                          ,--./,-.
          ___     __   __   __   ___     /,-._.--~\
    |\ | |__  __ /  ` /  \ |__) |__         }  {
    | \| |       \__, \__/ |  \ |___     \`-._,-`-,
                                          `._,._,'

    nf-core/tools version 2.7.1 - https://nf-co.re

DEBUG    Popen(['git', 'cat-file', '--batch-check'], cwd=/home/nvnieuwk/Documents/cmgg/nf-cmgg-germline, universal_newlines=False, shell=None,             cmd.py:931
         istream=<valid stream>)                                                                                                                                     
DEBUG    Using config file: .nf-core.yml                                                                                                                 utils.py:963
DEBUG    Got '.' as path                                                                                                                                 utils.py:215
DEBUG    Found a config cache, loading: /home/nvnieuwk/.nextflow/nf-core/wf-config-cache-653e8a139e1ec6c57ec37bfb4.json                                  utils.py:249
DEBUG    Popen(['git', 'version'], cwd=/home/nvnieuwk/.config/nfcore/nf-core/modules, universal_newlines=False, shell=None, istream=None)                  cmd.py:931
DEBUG    Popen(['git', 'fetch', '-v', '--progress', 'origin'], cwd=/home/nvnieuwk/.config/nfcore/nf-core/modules, universal_newlines=True, shell=None,     cmd.py:931
         istream=None)                                                                                                                                               
DEBUG    Popen(['git', 'checkout', 'master'], cwd=/home/nvnieuwk/.config/nfcore/nf-core/modules, universal_newlines=False, shell=None, istream=None)       cmd.py:931
DEBUG    Popen(['git', 'merge', 'origin/master'], cwd=/home/nvnieuwk/.config/nfcore/nf-core/modules, universal_newlines=False, shell=None, istream=None)   cmd.py:931
DEBUG    Using config file: /home/nvnieuwk/.config/nfcore/nf-core/modules/.nf-core.yml                                                                   utils.py:963
DEBUG    Using config file: .nf-core.yml                                                                                                                 utils.py:963
DEBUG    The following files were modified by prettier:                                                                                              lint_utils.py:79

         modules.json                                                                                                                                                

DEBUG    No lint config file found: ./.nf-core-lint.yaml                                                                                    components_command.py:182
INFO     Testing pipeline: .                                                                                                                          __init__.py:265
DEBUG    Running lint test: files_exist                                                                                                               __init__.py:328
DEBUG    Running lint test: nextflow_config                                                                                                           __init__.py:328
DEBUG    Running lint test: files_unchanged                                                                                                           __init__.py:328
DEBUG    Using config file: .nf-core.yml                                                                                                                 utils.py:963
ERROR    Invalid workflow name: must be lowercase without punctuation.                                                                                __main__.py:376
awgymer commented 1 year ago

Hmm maybe I borked something with .nf-core-lint.yaml there.

I will investigate this in detail and report back.

nvnieuwk commented 1 year ago

Thank you!! :partying_face:

awgymer commented 1 year ago

Oh no. I think I know what the issue is! I modified load_tools_config and it returns a tuple now. Most other places this broke anywhere that didn't get updated to reflect that, but because in PipelineCreate we short circuit, first checking "lint" in config_yml it is just never true because "lint" is never one of the values in the tuple.

Will fix later when I am back at my desk.

awgymer commented 1 year ago

I think I have the fix in #2122 but could you perhaps either test the branch or provide a minimum reproducible example I can run the code on to confirm @nvnieuwk

nvnieuwk commented 1 year ago

Can confirm it works now! Thank you! FYI this is the output:

$ nf-core lint

                                          ,--./,-.
          ___     __   __   __   ___     /,-._.--~\
    |\ | |__  __ /  ` /  \ |__) |__         }  {
    | \| |       \__, \__/ |  \ |___     \`-._,-`-,
                                          `._,._,'

    nf-core/tools version 2.8.dev0 - https://nf-co.re

INFO     Testing pipeline: .                                                                                                                                         

╭─ [?] 20 Pipeline Tests Ignored ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                                   │
│ files_exist: File is ignored: assets/nf-core-nfcmggstructural_logo_light.png                                                                                      │
│ files_exist: File is ignored: docs/images/nf-core-nfcmggstructural_logo_light.png                                                                                 │
│ files_exist: File is ignored: docs/images/nf-core-nfcmggstructural_logo_dark.png                                                                                  │
│ files_exist: File is ignored: .github/ISSUE_TEMPLATE/config.yml                                                                                                   │
│ files_exist: File is ignored: .github/workflows/awstest.yml                                                                                                       │
│ files_exist: File is ignored: .github/workflows/awsfulltest.yml                                                                                                   │
│ nextflow_config: Config variable ignored: manifest.name                                                                                                           │
│ nextflow_config: Config variable ignored: manifest.homePage                                                                                                       │
│ files_unchanged: File ignored due to lint config: .github/CONTRIBUTING.md                                                                                         │
│ files_unchanged: File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml                                                                           │
│ files_unchanged: File does not exist: .github/ISSUE_TEMPLATE/config.yml                                                                                           │
│ files_unchanged: File ignored due to lint config: .github/ISSUE_TEMPLATE/feature_request.yml                                                                      │
│ files_unchanged: File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md                                                                                │
│ files_unchanged: File ignored due to lint config: .github/workflows/linting.yml                                                                                   │
│ files_unchanged: File ignored due to lint config: assets/email_template.txt                                                                                       │
│ files_unchanged: File ignored due to lint config: assets/sendmail_template.txt                                                                                    │
│ files_unchanged: File ignored due to lint config: lib/NfcoreTemplate.groovy                                                                                       │
│ files_unchanged: File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml                                                                 │
│ actions_ci: actions_ci                                                                                                                                            │
│ actions_awstest: 'awstest.yml' workflow not found: ./.github/workflows/awstest.yml                                                                                │
│                                                                                                                                                                   │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
╭─ [!] 3 Pipeline Test Warnings ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                                   │
│ files_exist: File not found: lib/WorkflowNf-cmgg-germline.groovy                                                                                                  │
│ pipeline_name_conventions: Naming does not adhere to nf-core conventions: Contains non alphanumeric characters                                                    │
│ schema_description: No description provided in schema for parameter: vcfanno_lua                                                                                  │
│                                                                                                                                                                   │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
╭─ [✗] 3 Pipeline Tests Failed ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                                   │
│ files_unchanged: .gitattributes does not match the template                                                                                                       │
│ files_unchanged: .github/workflows/linting_comment.yml does not match the template                                                                                │
│ files_unchanged: lib/NfcoreSchema.groovy does not match the template                                                                                              │
│                                                                                                                                                                   │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

╭───────────────────────╮
│ LINT RESULTS SUMMARY  │
├───────────────────────┤
│ [✔] 265 Tests Passed  │
│ [?]  20 Tests Ignored │
│ [!]   3 Test Warnings │
│ [✗]   3 Tests Failed  │
╰───────────────────────╯

Tip: Some of these linting errors can automatically be resolved with the following command:

    nf-core lint   --fix files_unchanged
awgymer commented 1 year ago

Hmm. Is it supposed to still be a warning if it's being marked as skip? @mirpedrol

pipeline_name_conventions: Naming does not adhere to nf-core conventions: Contains non alphanumeric characters

nvnieuwk commented 1 year ago

I've managed to turn if of by adding pipeline_name_conventions: false to the .nf-core.yml under lint

nvnieuwk commented 1 year ago

But yeah it's maybe better to not add the warning if the manifest.name already isn't checked

mirpedrol commented 1 year ago

Oh, I thought we had this check for lint but not for create which was when the code failed. Thank you very much for the fix @awgymer. I agree with not adding the warning :)

mirpedrol commented 1 year ago

I was testing this now, and it fixes the issue with nf-core lint which now only prints the warning. But still fails with nf-core sync as it calls create() which doesn't take into account the .nf-core.yml file.