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

Workflow report #134

Closed
wants to merge 2 commits into from
Closed

Workflow report #134

wants to merge 2 commits into from

Conversation

muffato
Copy link
Member

@muffato muffato commented Aug 23, 2023

Closes #129

Hi @DLBPointon

In #132 I couldn't get workflow.onComplete to work at all, but even if I could run it, there are problems (cf #129) and the current implementation feels awkward in some regards:

  • Values are added to params during the workflow, which is not really recommended.
  • Values can't be retrieved the normal way - .value() needs to be added. Probably a consequence of the above !

Here I present an alternative way of running the summary function, simply through regular channel manipulations and function calls.

  1. All the data are collected with combine() in a channel.
  2. Then structured into a map with .map.
  3. The resulting channel is calledcollected_metrics_ch. By construction, it contains a single element.
  4. TreeValProject.summary is called on that map object, plus the workflow and params.

cc-ing @priyanka-surana for visibility

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

muffato added 2 commits August 23, 2023 20:31
@muffato
Alternative way of creating the report
4236993 
Instead of using workflow.onComplete (and battling to pass values through params),
collate the data with a regular channel and call the function directly.
@muffato
Added the log object to be able to log messages
736c7af
@muffato muffato requested a review from DLBPointon August 23, 2023 20:46
@github-actions
Copy link

github-actions bot commented Aug 23, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 736c7af

+| ✅ 126 tests passed       |+
#| ❔  16 tests were ignored |#
!| ❗   9 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • nextflow_config - Config manifest.version should end in dev: '1.0.0'
  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).
  • pipeline_todos - TODO string in methods_description_template.yml: ## Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release
  • pipeline_todos - TODO string in ci.yml: You can customise CI pipeline run tests as required
  • system_exit - System.exit in WorkflowTreeval.groovy: System.exit(1) [line 17]

❔ Tests ignored:

  • files_exist - File is ignored: assets/nf-core-treeval_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-treeval_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-treeval_logo_dark.png
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • 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 ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File does not exist: assets/nf-core-treeval_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-treeval_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-treeval_logo_dark.png
  • 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_awstest - 'awstest.yml' workflow not found: /home/runner/work/treeval/treeval/.github/workflows/awstest.yml

✅ Tests passed:

Run details

  • nf-core/tools version 2.8
  • Run at 2023-08-23 20:51:07

@muffato
Copy link
Member Author

muffato commented Aug 23, 2023 •
edited
Loading

By the way, at some point I managed to get an error that got Nextflow to print what TreeValProject.summary was getting when called from workflow.onComplete. See below (indentation mine), there is no trace of rf_data & co !

ERROR ~ Invalid method invocation `summary` with arguments: repository: null,
[
	projectDir: /lustre/scratch123/tol/teams/tolit/users/mm49/nextflow/src/sanger-tol/treeval,
	commitId: null,
	revision: null,
	startTime: 2023-08-23T20:14:25.656623+01:00,
	endTime: 2023-08-23T20:14:45.650657+01:00,
	duration: 20s,
	container: {},
	commandLine: nextflow run src/sanger-tol/treeval -profile test,singularity -config src/configs/publish_all_outputs.config --input src/sanger-tol/treeval/assets/local_testing/nxOscSUBSET.yaml -entry RAPID --outdir results_rapid --max_cpus 8 --max_memory 48GB -resume ridiculous_meitner,
	nextflow: nextflow.NextflowMeta(
		version:23.04.1,
		build:5866,
		timestamp:15-04-2023 06:51 UTC,
		preview:nextflow.NextflowMeta$Preview@49fb693d,
		enable:nextflow.NextflowMeta$Features@38197e82,
		dsl2:true,
		dsl2Final:true,
		strictModeEnabled:false
	),
	success: true,
	workDir: /lustre/scratch123/tol/teams/tolit/users/mm49/nextflow/work,
	launchDir: /lustre/scratch123/tol/teams/tolit/users/mm49/nextflow,
	profile: test,singularity
] (nextflow.script.WorkflowMetadata),
[
	input:src/sanger-tol/treeval/assets/local_testing/nxOscSUBSET.yaml,
	outdir:results_rapid,
	max_cpus:8,
	max_memory:48GB,
	tracedir:results_rapid/treeval_info,
	publish_dir_mode:copy,
	email:null,
	email_on_fail:null,
	plaintext_email:false,
	monochrome_logs:false,
	hook_url:null,
	help:false,
	version:false,
	validate_params:true,
	show_hidden_params:false,
	schema_ignore_params:genomes,
	custom_config_version:master,
	custom_config_base:https://raw.githubusercontent.com/nf-core/configs/master,
	config_profile_description:Minimal test dataset to check pipeline function,
	config_profile_contact:null,
	config_profile_url:null,
	config_profile_name:Test profile,
	max_time:6.h,
	trace_timestamp:2023-08-23_20-14-24
] (nextflow.script.ScriptBinding$ParamsMap) on TreeValProject type

@DLBPointon
Copy link
Contributor

Hi @muffato,
Sorry for the late reply, annual leave.
This does indeed look like a much more elegant solution, thank you. Once the HPC is running again, i'll run some testing.

@DLBPointon
Copy link
Contributor

I've added your changes to #132. I didn't do it through a merge but have attributed the authorship to you so let me know if there's anything wrong with it.
Thanks.

@DLBPointon DLBPointon closed this Sep 7, 2023
@DLBPointon DLBPointon deleted the workflow_report branch September 19, 2023 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DLBPointon DLBPointon Awaiting requested review from DLBPointon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@muffato @DLBPointon