Skip to content

Fix bug for kraken2/bracken in multiqc#616

Merged
sofstam merged 8 commits into
nf-core:devfrom
sofstam:fix_bracken_nanopore
Jul 2, 2025
Merged

Fix bug for kraken2/bracken in multiqc#616
sofstam merged 8 commits into
nf-core:devfrom
sofstam:fix_bracken_nanopore

Conversation

@sofstam

@sofstam sofstam commented Jun 5, 2025

Copy link
Copy Markdown
Collaborator

PR checklist

Closes #612

  • 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
  • If necessary, also make a PR on the nf-core/taxprofiler branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,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).

@nf-core-bot

nf-core-bot commented Jun 5, 2025

Copy link
Copy Markdown
Member

Warning

Newer version of the nf-core template is available.

Your pipeline is using an old version of the nf-core template: 3.2.1.
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.

@sofstam sofstam marked this pull request as draft June 5, 2025 13:36
@jfy133

jfy133 commented Jun 26, 2025

Copy link
Copy Markdown
Member

@nf-core-bot fix linting

@jfy133

jfy133 commented Jun 26, 2025

Copy link
Copy Markdown
Member

@sofstam could you 'review' your own PR please and test, if it looks good I can ✔️ on your behalf 😬

@jfy133 jfy133 marked this pull request as ready for review June 26, 2025 09:11
@jfy133

jfy133 commented Jun 26, 2025

Copy link
Copy Markdown
Member

The code changes I made are on lines 160-197 (sorry it's so messsy... Nextflow autofromatter...)

@jfy133

jfy133 commented Jun 26, 2025

Copy link
Copy Markdown
Member

I confirm test remains looking good:

[9d/909e4f] NFCORE_TAXPROFILER:TAXPROFILER:PROFILING:KRAKEN2_KRAKEN2 (db2|kraken2|2612)                             [100%] 6 of 6 ✔
[e1/bc90d8] NFCORE_TAXPROFILER:TAXPROFILER:PROFILING:BRACKEN_BRACKEN (db1|2612)                                     [100%] 3 of 3 ✔

So we've not broken anything.

Where we have 3 (merged) Illumina samples, and 2 kraken databases and 1 bracken database :)

@sofstam

sofstam commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator Author

I tested and I confirm that it works as it should. I tested to have both short and long db_type for kraken2 and multiqc looks like:

Screenshot 2025-07-01 at 15 18 38 Screenshot 2025-07-01 at 15 19 25

@sofstam

sofstam commented Jul 1, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

I cannot approve since I opened the PR but maybe you can approve @jfy133?
Shall we wait with merging to fix the nf-test first?

@jfy133

jfy133 commented Jul 1, 2025

Copy link
Copy Markdown
Member

No, we can get this in the meantime!

jfy133
jfy133 approved these changes Jul 1, 2025
View reviewed changes

@jfy133 jfy133 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just need a change log update then you can merge @sofstam !

jfy133
jfy133 approved these changes Jul 1, 2025
View reviewed changes
Comment thread CHANGELOG.md Outdated
@jfy133

jfy133 commented Jul 1, 2025

Copy link
Copy Markdown
Member

@sofstam you forgot yourself as we did this together. You can then merge!

@sofstam @jfy133
Update CHANGELOG.md
07fd4d2 
Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>
@sofstam

sofstam commented Jul 2, 2025

Copy link
Copy Markdown
Collaborator Author

@nf-core-bot fix linting

@sofstam sofstam merged commit 374145e into nf-core:dev Jul 2, 2025
4 of 7 checks passed
@sofstam sofstam mentioned this pull request Jul 3, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jfy133 jfy133 jfy133 approved these changes

@LilyAnderssonLee LilyAnderssonLee Awaiting requested review from LilyAnderssonLee LilyAnderssonLee is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sofstam @nf-core-bot @jfy133