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

Move import in utils_nfcore_phaseimpute_pipeline #3476

Closed
wants to merge 4 commits into from

Conversation

LouisLeNezet
Copy link
Contributor

@LouisLeNezet LouisLeNezet commented Mar 3, 2025

PR checklist

This PR should remove warning in nf-core pipelines lint for the different import use in the email workflow.

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

Co-authored-by: Matthias Hörtenhuber <[email protected]>
@LouisLeNezet LouisLeNezet requested a review from mashehu March 3, 2025 15:55
@LouisLeNezet
Copy link
Contributor Author

Is it good now @mashehu ?

Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @LouisLeNezet, when do you see this warning? and could you explain why is this change solving it? Thanks!

@LouisLeNezet
Copy link
Contributor Author

LouisLeNezet commented Mar 5, 2025

Hi,

Here is the warning that I can get by running nf-core pipelines lint on the TEMPLATE branch of phaseimpute.

╭─ [!] 16 Subworkflow Test Warnings ────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│                                    ╷                                                      ╷                                                       │
│ Subworkflow name                   │ File path                                            │ Test message                                          │
│╶───────────────────────────────────┼──────────────────────────────────────────────────────┼──────────────────────────────────────────────────────╴│
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'completionEmail' not used in      │
│                                    │                                                      │ main.nf                                               │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'completionSummary' not used in    │
│                                    │                                                      │ main.nf                                               │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'imNotification' not used in       │
│                                    │                                                      │ main.nf                                               │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'paramsSummaryMap' not used in     │
│                                    │                                                      │ main.nf                                               │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'UTILS_NEXTFLOW_PIPELINE' versions │
│                                    │                                                      │ are not added in main.nf                              │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'UTILS_NFCORE_PIPELINE' versions   │
│                                    │                                                      │ are not added in main.nf                              │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'UTILS_NFSCHEMA_PLUGIN' versions   │
│                                    │                                                      │ are not added in main.nf                              │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'completionEmail' versions are not │
│                                    │                                                      │ added in main.nf                                      │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'completionSummary' versions are   │
│                                    │                                                      │ not added in main.nf                                  │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'imNotification' versions are not  │
│                                    │                                                      │ added in main.nf                                      │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'paramsSummaryMap' versions are    │
│                                    │                                                      │ not added in main.nf                                  │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Included component 'samplesheetToList' versions are   │
│                                    │                                                      │ not added in main.nf                                  │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ Subworkflow meta.yml does not exist                   │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ TODO string in main.nf: Only uncomment below if logic │
│                                    │                                                      │ in toolCitationText/toolBibliographyText has been     │
│                                    │                                                      │ filled!                                               │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ TODO string in main.nf: Optionally add bibliographic  │
│                                    │                                                      │ entries to this list.                                 │
│ utils_nfcore_phaseimpute_pipeline  │ subworkflows/local/utils_nfcore_phaseimpute_pipelin… │ TODO string in main.nf: Optionally add in-text        │
│                                    │                                                      │ citation tools to this list.                          │
│                                    ╵                                                      ╵                                                       │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

I don't really know how this linting is done but just by moving the import above the concerned workflow solved the issue. So we might either keep the importation near the workflow or change the linting to check the import for all workflow scope in the main.nf.

@mirpedrol
Copy link
Member

I can see the warning in phaseimpute but not in other pipelines, so the error must be somewhere else, the location of imports should always be at the top of the file

@LouisLeNezet
Copy link
Contributor Author

The other issue is that in many pipelines the linting of subworkflows is deactivated as they've added lines in the .nf-core.yml

@mirpedrol
Copy link
Member

Found the error. This line should be
if lint_obj.lint_config and lint_obj.lint_config["nfcore_components"] is not None:, because we set all lint_config fields to None by default

@mirpedrol
Copy link
Member

This local subworkflow is a bit different because we have several workflows, usually we only have 1 workflow per subworkflow. I would modify the main_nf() function, it should take into account all the workflows when checking the included components

@LouisLeNezet
Copy link
Contributor Author

Okay so I will close this PR. Should I open a new issue regarding the default linting and the main.nf() ?

@mirpedrol
Copy link
Member

That would be great, thanks!

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

Successfully merging this pull request may close these issues.

4 participants