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.

Sign up for GitHub

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

Jump to bottom

Cleanup needed for nf-core logo / monochrome logs. #3295

Open
ewels opened this issue Nov 23, 2024 · 2 comments · May be fixed by nf-core/modules#7094
Open

Cleanup needed for nf-core logo / monochrome logs. #3295

ewels opened this issue Nov 23, 2024 · 2 comments · May be fixed by nf-core/modules#7094
Labels
template nf-core pipeline/component template
Milestone
3.1

Comments

@ewels
Copy link
Member

ewels commented Nov 23, 2024

Started looking into warnings for #3294 and saw one about monochrome_logs being unused here:

tools/nf_core/pipeline-template/subworkflows/local/utils_nfcore_pipeline_pipeline/main.nf

Line 35 in 41b4516

monochrome_logs // boolean: Do not use coloured log outputs

This seems to be a leftover from before we moved to nf-schema to print the header. The nf-core utils subworkflow no longer does this, so no longer needs the monochrome_logs option to be passed to it.

The more I started looking into it, the more I think that there is a bit of cleanup needed here.

  • nf-schema has a configuration option called validation.monochromeLogs
    • We're not setting this currently, we probably should - set to params.monochrome_logs
  • We have def nfCoreLogo in the nf-core subworkflow which is, as far as I'm aware, not used anywhere
  • Pretty sure that the dashedLine function is only used by nfCoreLogo, so can also be deleted.
  • The nextflow.config hardcodes the logo (not using the above function), with ANSI colours hardcoded. So there is no way to disable colours.

As a minimum we need to remove the unused input from the subworkflow to help solve #3294 - but we should also try to clean up this 👆🏻 so that:

  • --monochrome_logs makes the nf-core logo monochrome
  • There is minimal code duplication / orphaned unused code
@ewels ewels added the template nf-core pipeline/component template label Nov 23, 2024
@ewels ewels added this to the 3.1 milestone Nov 23, 2024
@ewels
Copy link
Member Author

ewels commented Nov 23, 2024

Unresolved - how to fix this:

tools/nf_core/pipeline-template/nextflow.config

Lines 252 to 255 in 41b4516

def trace_timestamp = new java.util.Date().format( 'yyyy-MM-dd_HH-mm-ss')
timeline {
enabled = true
file = "${params.outdir}/pipeline_info/execution_timeline_${trace_timestamp}.html"

We're not allowed def in nextflow.config any more.

We originally added it years ago to avoid filename clashes, but that's no longer needed. However, I'm not sure how to set the report directories to "${params.outdir}/pipeline_info/ without also specifying the report filename. And if I specify the report filename I don't know how to include the timestamp 🫠

So we need to either:

  • Somehow specify the report directory and leave the default Nextflow filename (not possible?)
  • Refactor the above code to give the entire file path, including a timestamp

@mirpedrol
Copy link
Member

mirpedrol commented Nov 25, 2024 •
edited
Loading

using validation.monochromeLogs will remove the colours from the logo we have hard coded in validation.beforeText. See the nf-schema code here.
But it doesn't work when setting validation.monochromeLogs = params.monochrome_logs. This looks like one of the Nextflow particularities when overriding params 🤔 do you have an idea why this could be happening? It does work 😄

@mirpedrol mirpedrol linked a pull request Nov 26, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
template nf-core pipeline/component template
Projects
None yet
Milestone
3.1
Development

Successfully merging a pull request may close this issue.

Remove unused code from utils_nfcore_pipeline swf mirpedrol/modules
2 participants
@ewels @mirpedrol