Skip to content

Update for btk plots#230

Open
DLBPointon wants to merge 35 commits intodevfrom
plots
Open

Update for btk plots#230
DLBPointon wants to merge 35 commits intodevfrom
plots

Conversation

@DLBPointon
Copy link
Copy Markdown
Contributor

@DLBPointon DLBPointon commented Mar 30, 2026 •
edited
Loading

I was having a look at the plots module.

A slightly unintuitive part of it is that you need the module.config for it to work. Sent me in circles for a bit before i remembered.

I've not updated the tests yet, I want your input to see if anything else should be done first.

muffato and others added 20 commits March 23, 2026 20:48
@muffato
nf-core module update
cf77290
@muffato
Use the new sanger-tol module for that
b3503ff
@muffato
No need to lint sanger-tol modules and sub-workflows
0bab621
@muffato
We require a recent plugin to download assets
41170e8
@muffato
Snapshot update
ec41c32
@muffato
Updated all remaining nf-core modules
f00478b 
Except multiqc because its inferface has changed and I'd rather wait
for the official template udpate
@muffato
Snapshot update
86cc6e6
@muffato
Use the new repeat-masking subworkflow
538721a
@muffato
Clarification
37e29ff
@muffato
Combined the sub-workflows in a way that makes more sense
30b1ef1
@muffato
This file now differs from the template
87ff33e
@muffato
Move to the upstream blobtk/depth
46cb0cd
@muffato
With no versions.yml emitted, we can simplify the publish rules
10177fe
@muffato
Only publish the HTML file
1b404ee
@muffato
tag can be set dynamically from the config file
a396983
@muffato
Name the BUSCO outputs with the accession number and lineage name
9dbd2d2
@DLBPointon
Update for btk plots
2c2640a
@DLBPointon
Add plots
fa81cff
@DLBPointon
Delete old module
4499c11
@DLBPointon
Update for blobtk_plots
d6e25d5
@DLBPointon DLBPointon marked this pull request as draft March 30, 2026 16:25
@DLBPointon DLBPointon moved this from Todo to In progress in WRAP team board Mar 30, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 30, 2026 •
edited
Loading

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 9af5cfb

+| ✅ 202 tests passed       |+
#| ❔  30 tests were ignored |#
!| ❗  18 tests had warnings |!
Details

❗ Test warnings:

❔ Tests ignored:

  • files_exist - File is ignored: CODE_OF_CONDUCT.md
  • files_exist - File is ignored: assets/nf-core-blobtoolkit_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-blobtoolkit_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-blobtoolkit_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
  • files_exist - File is ignored: conf/igenomes.config
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: .gitattributes
  • files_unchanged - File ignored due to lint config: CODE_OF_CONDUCT.md
  • 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 does not exist: .github/ISSUE_TEMPLATE/config.yml
  • files_unchanged - File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md
  • files_unchanged - File ignored due to lint config: .github/workflows/branch.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting_comment.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/email_template.html
  • 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: assets/nf-core-blobtoolkit_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-blobtoolkit_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-blobtoolkit_logo_dark.png
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/blobtoolkit/blobtoolkit/.github/workflows/awstest.yml
  • template_strings - template_strings
  • merge_markers - merge_markers

✅ Tests passed:

Run details

  • nf-core/tools version 3.5.2
  • Run at 2026-04-30 15:52:47

@DLBPointon DLBPointon requested a review from muffato April 1, 2026 09:29
@DLBPointon
Copy link
Copy Markdown
Contributor Author

DLBPointon commented Apr 1, 2026

nf-core/modules#11105

muffato
muffato reviewed Apr 11, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@muffato muffato left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

Not sure why only the blob image generation is failing.
Also, we can only notice because the nf-test snapshots differ. Failures are otherwise hidden by the errorStrategy 'ignore' line from modules/nf-core/blobtk/plot/main.nf. I wouldn't want that behaviour in production.
If I remember correctly, you only had the problem with some remote URLs, right ? Our local module for blobtk never failed so far, so I think it's OK to patch the module and remove that custom errorStrategy.

Comment thread conf/modules.config
@DLBPointon
Copy link
Copy Markdown
Contributor Author

DLBPointon commented Apr 13, 2026

Absolutely doesn't want to run the blob

Command executed:

  blobtk plot \
      -d GCA_922984935.2 \
      -o GCA_922984935.2_blob.png \
      --legend full \
      --scale-factor 0.6 -v blob

Command exit status:
  134

Command output:
  (empty)

Command error:

  thread 'main' panicked at src/plot/component.rs:1294:41:
  index out of bounds: the len is 0 but the index is 0
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
  .command.sh: line 6:    42 Aborted                 blobtk plot -d GCA_922984935.2 -o GCA_922984935.2_blob.png --legend full --scale-factor 0.6 -v blob

@muffato
Copy link
Copy Markdown
Member

muffato commented Apr 13, 2026

:/ One for Rich, I guess

@muffato muffato force-pushed the module_update branch 2 times, most recently from 222495b to 3b3a692 Compare April 13, 2026 17:22
Base automatically changed from module_update to dev April 14, 2026 08:12
@sanger-tolsoft
Copy link
Copy Markdown
Contributor

sanger-tolsoft commented Apr 29, 2026

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.5.2.
Please update your pipeline to the latest version.

For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation.

muffato
muffato reviewed Apr 29, 2026
View reviewed changes
Comment thread workflows/blobtoolkit.nf Outdated
DLBPointon and others added 2 commits April 29, 2026 17:23
@DLBPointon @muffato
Update workflows/blobtoolkit.nf
d2e78af 
Co-authored-by: Matthieu Muffato <mm49@sanger.ac.uk>
@DLBPointon
Linting
69d1524
@muffato
Copy link
Copy Markdown
Member

muffato commented Apr 29, 2026

Hopefully this only needs the snapshots to be updated and we're good to go.

I've already reviewed the files and I'm generally OK. Just want to double-check if/how the snapshots end up changing with this PR, to validate subworkflows/local/view.nf (I only expect the blobtk version numbers to be changing).

@DLBPointon
Copy link
Copy Markdown
Contributor Author

DLBPointon commented Apr 30, 2026 •
edited
Loading

@muffato I'm not sure why just the test profile is failing. I think because it is relying on such a trimmed dataset, but then surely it would have failed before. My runs are always producing the same, github is always producing the same. You have any insight at all?

@muffato muffato marked this pull request as ready for review April 30, 2026 12:03
Copilot AI review requested due to automatic review settings April 30, 2026 12:03
@muffato
Copy link
Copy Markdown
Member

muffato commented Apr 30, 2026

Yes, we've recently noticed that BUSCO results weren't stable. Not sure why, I thought they were ? Anyway, the test passed after restarting.

muffato
muffato reviewed Apr 30, 2026
View reviewed changes
Comment thread tests/default.nf.test.snap
"vertebrata_odb10_count_windows.json.gz:md5,bd32fe8d54213687ecb94ac3f2cbce59",
"vertebrata_odb10_count_windows_100000.json.gz:md5,36d6694edc4979b34e0a7a5d1212ecdd",
"vertebrata_odb10_count_windows_1000000.json.gz:md5,40f404060aacde76cdf1f4a399daf270",
"GCA_922984935.2.blob.png:md5,380cbd5d56093d4a4673f22217bf5c35",
Copy link
Copy Markdown
Member

@muffato muffato Apr 30, 2026

Choose a reason for hiding this comment

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

Ah wait, maybe that's what you meant ?
The blob image not being generated any more is a regression in blobtk. That's something for Rich to look into.

Secondly, errorStrategy 'ignore' needs to be removed from the module. It's too dangerous to use in a production pipeline

Copilot AI reviewed Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the pipeline’s static plot generation to use an nf-core blobtk/plot module (and blobtk v0.8.0), aligning plotting behavior with the modules ecosystem and refreshing test snapshots accordingly.

Changes:

  • Replaced the local BLOBTK_IMAGES usage in VIEW with the nf-core BLOBTK_PLOT module.
  • Added a new nf-core module implementation + nf-test coverage for blobtk/plot, and updated blobtk/depth to v0.8.0.
  • Updated pipeline/module configs and nf-test snapshots to reflect new outputs and version reporting.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/raw.nf.test.snap Updates expected outputs/versions and plot checksums for raw profile.
tests/nobusco.nf.test.snap Updates expected outputs/versions and plot checksums for nobusco profile.
tests/default.nf.test.snap Updates expected outputs/versions for default profile (notably plot output expectations).
subworkflows/local/view.nf Switches plotting from BLOBTK_IMAGES to nf-core BLOBTK_PLOT and reshapes inputs.
subworkflows/local/run_blastn.nf Comment/header formatting only.
subworkflows/local/minimap_alignment.nf Comment/header formatting only.
subworkflows/local/finalise_blobdir.nf Minor whitespace formatting.
subworkflows/local/coverage_stats.nf Comment/header formatting and section labeling.
subworkflows/local/collate_stats.nf Comment/header formatting and section labeling.
subworkflows/local/blobtools.nf Reformatting multi-line module calls for readability.
modules/nf-core/blobtk/plot/tests/nextflow.config Adds module test config to supply args/prefix via ext.*.
modules/nf-core/blobtk/plot/tests/main.nf.test.snap Adds initial snapshots for the new blobtk/plot module tests.
modules/nf-core/blobtk/plot/tests/main.nf.test Adds nf-test cases for png/svg/stub behavior of BLOBTK_PLOT.
modules/nf-core/blobtk/plot/meta.yml Adds nf-core-style metadata for the new blobtk/plot module.
modules/nf-core/blobtk/plot/main.nf Implements the new BLOBTK_PLOT process (blobtk v0.8.0).
modules/nf-core/blobtk/plot/environment.yml Adds conda environment pin for blobtk=0.8.0.
modules/nf-core/blobtk/depth/tests/main.nf.test.snap Updates snapshots for blobtk depth version reporting and metadata.
modules/nf-core/blobtk/depth/tests/main.nf.test Adjusts depth module tests to provide args via params/config.
modules/nf-core/blobtk/depth/meta.yml Fixes tool metadata naming/identifier to blobtk.
modules/nf-core/blobtk/depth/main.nf Updates container to blobtk v0.8.0 and removes legacy versions.yml generation.
modules/nf-core/blobtk/depth/environment.yml Pins conda dependency to blobtk=0.8.0.
modules/local/blobtoolkit/createblobdir.nf Minor tag string interpolation adjustment.
modules.json Records installation/update of blobtk/depth and adds blobtk/plot.
conf/modules.config Adds BLOBTK_PLOT publishDir + ext.args/ext.prefix wiring; removes old BLOBTK_IMAGES block.
conf/base.config Minor whitespace formatting.
CHANGELOG.md Notes module swap and updates blobtk version info in the changelog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modules/nf-core/blobtk/plot/meta.yml
Comment thread CHANGELOG.md Outdated
Comment thread tests/default.nf.test.snap
@@ -334,7 +334,6 @@
"blobtoolkit/GCA_922984935.2/vertebrata_odb10_count_windows_100000.json.gz",
"blobtoolkit/GCA_922984935.2/vertebrata_odb10_count_windows_1000000.json.gz",
"blobtoolkit/plots",
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

The default pipeline snapshot no longer expects blobtoolkit/plots/GCA_922984935.2.blob.png, even though VIEW still requests a blob plot. This makes it easy for a regression in blob plot generation to go unnoticed; if the intent is to still generate the blob plot, prefer fixing the underlying BLOBTK_PLOT failure rather than updating the snapshot to accept a missing output.

Suggested change
"blobtoolkit/plots",
"blobtoolkit/plots",
"blobtoolkit/plots/GCA_922984935.2.blob.png",

Copilot uses AI. Check for mistakes.
Comment thread modules/nf-core/blobtk/plot/main.nf
Comment thread modules/nf-core/blobtk/plot/main.nf
Comment thread modules/nf-core/blobtk/plot/main.nf
Comment on lines +2 to +12
// Linked to issue https://github.com/sanger-tol/genomenote/issues/184
// Depending on the blob dataset in use, the grid option may not
// work at all. This is down to the version of blobtoolkit used to
// generate the blob.
// Adding a check would overly complicate the module so for now
// we can ignore errors, with the knowledge it would only kill
// runs in which the blobdir doesn't have the right data.
errorStrategy 'ignore'

tag "$prefix"
label 'process_single'
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

errorStrategy 'ignore' can silently drop expected plot outputs and hide real failures (e.g., network issues, invalid args). The updated pipeline snapshot now no longer contains the blob plot for the default test profile, which is consistent with a failure being ignored; consider narrowing the ignore behaviour to specific known exit codes/messages and/or emitting an explicit warning when a plot is skipped.

Copilot uses AI. Check for mistakes.
Comment thread subworkflows/local/view.nf
@muffato
typo
9af5cfb 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@DLBPointon DLBPointon linked an issue May 1, 2026 that may be closed by this pull request
Update blobtk/plots to sanger-tol version #222
Open
@DLBPointon
Copy link
Copy Markdown
Contributor Author

DLBPointon commented May 5, 2026

Just tested Rich's 0.8.1 for blobtk, works on the faulty blobdir. Just have to wait on bioconda accepting it (bioconda/bioconda-recipes#65014) and then I'll update nf-core again.

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

Reviewers

@muffato muffato muffato left review comments

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

WRAP team board
Status: In progress

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Update blobtk/plots to sanger-tol version

4 participants

@DLBPointon @muffato @sanger-tolsoft