Skip to content
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

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

Closed
nvnieuwk opened this issue Dec 9, 2022 · 14 comments · Fixed by #2122
Closed

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

nvnieuwk opened this issue Dec 9, 2022 · 14 comments · Fixed by #2122
Assignees
Milestone

Comments

@nvnieuwk
Copy link
Contributor

nvnieuwk commented Dec 9, 2022

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!!

@mirpedrol mirpedrol added this to the 2.7.2 milestone Dec 9, 2022
@awgymer
Copy link
Contributor

awgymer commented Dec 9, 2022

I thought we added code to skip this:

tools/nf_core/create.py

Lines 170 to 180 in eecf1c7

if (
"lint" in config_yml
and "nextflow_config" in config_yml["lint"]
and "manifest.name" in config_yml["lint"]["nextflow_config"]
):
return param_dict, skip_paths
# Check that the pipeline name matches the requirements
if not re.match(r"^[a-z]+$", param_dict["short_name"]):
raise UserWarning("[red]Invalid workflow name: must be lowercase without punctuation.")
return param_dict, skip_paths

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
Copy link
Contributor Author

nvnieuwk commented Dec 9, 2022

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
Copy link
Contributor

awgymer commented Dec 9, 2022

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
Copy link
Contributor Author

nvnieuwk commented Dec 9, 2022

$ 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
Copy link
Contributor

awgymer commented Dec 9, 2022

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

I will investigate this in detail and report back.

@nvnieuwk
Copy link
Contributor Author

nvnieuwk commented Dec 9, 2022

Thank you!! 🥳

@awgymer
Copy link
Contributor

awgymer commented Dec 9, 2022

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 awgymer self-assigned this Dec 9, 2022
@awgymer awgymer mentioned this issue Dec 9, 2022
4 tasks
@awgymer
Copy link
Contributor

awgymer commented Dec 9, 2022

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
Copy link
Contributor Author

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
Copy link
Contributor

awgymer commented Dec 12, 2022

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
Copy link
Contributor Author

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

@nvnieuwk
Copy link
Contributor Author

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

@mirpedrol
Copy link
Member

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants