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

feat: update multiqc to 1.22.3 #1441

Merged
merged 45 commits into from
Jun 26, 2024
Merged

feat: update multiqc to 1.22.3 #1441

merged 45 commits into from
Jun 26, 2024

Conversation

mathiasbio
Copy link
Contributor

@mathiasbio mathiasbio commented May 29, 2024

Description

The new version of multiqc supports picard mimicked reports from Sentieon tools: MultiQC/MultiQC#2110

This should solve this issue: #1290 where an ugly solution was implemented in the Dedup rule to make MultiQC accept dedup-stats from Sentieon dedup.

It may also allow us to move away from Picard to generate our QC reports and instead use the Sentieon tools which should be faster and enable us to clear away some rules for a more streamlined and less messy workflow.

Added

  • separate container for multiqc

Changed

  • updated multiqc from 1.12 to 1.22.3

Removed

  • no longer necessary sed command in dedup rule
  • deprecated and unused TNhaplotyper rule

Documentation

  • N/A
  • Updated Balsamic documentation to reflect the changes as needed for this PR.
    • read the docs: bioinfo_softwares

Tests

I downloaded the new multiqc docker (v1.22.3 of multiqc) as a singularity image and placed in a separate develop cache which I then used in my tests along with the updated align_qc.sif container which doesn't contain multiqc anymore

Ensure that we get all QC metrics in deliverables as before

NOTE: regarding multiqc version

  • As of 2024-04-31 it seems that there's a bug in multiqc or a feature, I don't know, but the latest version 1.22.1 does not pick up the relatedness score from somalier which kind of blocks this feature at the moment. But I have opened up a bug report in multiqc repo: updated multiqc no longer reports relatedness from somalier MultiQC/MultiQC#2596 and have tried downgrading the version to 1.18 which seems to be working.

  • As of 2024-06-05 They solved the bug and I have tested the latest version 1.22.2 which does pick up Somalier results, but I discovered that it also does not manage to pick up the Picard WgsMetrics results, which v1.18 does. So I will return to that older version. I opened a bug report CollectWgsMetrics from Picard not picked up in latest version MultiQC/MultiQC#2607

  • As of 2024-06-22 they are supposed to have solved the bug with version 1.22.3

  • Verified successfully for WGS T+N, and TGA-UMI T+N all metrics in the deliverables file are still there as in version 15.0.0

Feature Tests

  • Unaltered dedup metrics file from Sentieon dedup is picked up by multiqc. However! the name of the tool in multiqc is still referred to as Picard, not Sentieon. But at least the raw metric file is unaltered and can be traced to Sentieon.

Pipeline Integrity Tests

  • Report deliver (generation of the .hk file)
    • N/A
    • Verified
  • TGA T/O Workflow
    • N/A
    • Verified
  • TGA T/N Workflow
    • N/A
    • Verified
  • UMI T/O Workflow
    • N/A
    • Verified
  • UMI T/N Workflow
    • N/A
    • Verified
  • WGS T/O Workflow
    • N/A
    • Verified
  • WGS T/N Workflow
    • N/A
    • Verified
  • QC Workflow
    • N/A
    • Verified
  • PON Workflow
    • N/A
    • Verified

Clinical Genomics Stockholm

Documentation

  • Atlas documentation
    • N/A
    • Updated: [Link]
  • Web portal for Clinical Genomics
    • N/A
    • Updated: [Link]

User Changes

  • N/A
  • This PR affects the output files or results.
    • User feedback is considered unnecessary because [Justification].
    • Affected users have been included in the development process and given a chance to provide feedback.

Infrastructure Changes

  • Stored files in Housekeeper
    • N/A
    • Updated: [Link]
  • CG (CLI and delivered/uploaded files)
    • N/A
    • Updated: [Link]
  • Servers (configuration files on Hasta)
    • N/A
    • Updated: [Link]
  • Scout interface
    • N/A
    • Updated: [Link]

Checklist

Important

Ensure that all checkboxes below are ticked before merging.

For Developers

  • PR Description
    • Provided a comprehensive description of the PR.
    • Linked relevant user stories or issues to the PR.
  • Documentation
    • Verified and updated documentation if necessary.
  • Tests
    • Described and tested the functionality addressed in the PR.
    • Ensured integration of the new code with existing workflows.
    • Confirmed that meaningful unit tests were added for the changes introduced.
    • Checked that the PR has successfully passed all relevant code smells and coverage checks.
  • Review
    • Addressed and resolved all the feedback provided during the code review process.
    • Obtained final approval from designated reviewers.

For Reviewers

  • Code
    • Code implements the intended features or fixes the reported issue.
    • Code follows the project's coding standards and style guide.
  • Documentation
    • Pipeline changes are well-documented in the CHANGELOG and relevant documentation.
  • Tests
    • The author provided a description of their manual testing, including consideration of edge cases and boundary
      conditions where applicable, with satisfactory results.
  • Review
    • Confirmed that the developer has addressed all the comments during the code review.

@mathiasbio mathiasbio changed the base branch from master to develop May 29, 2024 09:37
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.48%. Comparing base (7d529e6) to head (f222381).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1441   +/-   ##
========================================
  Coverage    99.48%   99.48%           
========================================
  Files           40       40           
  Lines         1932     1933    +1     
========================================
+ Hits          1922     1923    +1     
  Misses          10       10           
Flag Coverage Δ
unittests 99.48% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mathiasbio mathiasbio linked an issue May 29, 2024 that may be closed by this pull request
3 tasks
@mathiasbio mathiasbio added this to the Release 16 milestone May 29, 2024
@mathiasbio mathiasbio self-assigned this May 29, 2024
@mathiasbio mathiasbio changed the title feat: update multiqc to 1.22.1 feat: update multiqc to 1.18 May 31, 2024
@mathiasbio mathiasbio marked this pull request as ready for review June 3, 2024 10:54
@mathiasbio mathiasbio requested a review from a team as a code owner June 3, 2024 10:54
@mathiasbio mathiasbio changed the base branch from update_sentieon to deduplicate_with_umi June 25, 2024 08:09
@mathiasbio mathiasbio changed the base branch from deduplicate_with_umi to update_sentieon June 25, 2024 08:10
@mathiasbio mathiasbio changed the title feat: update multiqc to 1.18 feat: update multiqc to 1.22.3 Jun 25, 2024
@mathiasbio mathiasbio changed the base branch from update_sentieon to develop June 25, 2024 14:51
@mathiasbio mathiasbio marked this pull request as ready for review June 25, 2024 15:07
Copy link
Contributor

@ivadym ivadym left a comment

Choose a reason for hiding this comment

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

Fantastic work ⭐

BALSAMIC/containers/align_qc/align_qc.yaml Show resolved Hide resolved
docs/bioinfo_softwares.rst Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
BALSAMIC/assets/scripts/collect_qc_metrics.py Outdated Show resolved Hide resolved
BALSAMIC/assets/scripts/collect_qc_metrics.py Outdated Show resolved Hide resolved
BALSAMIC/assets/scripts/collect_qc_metrics.py Outdated Show resolved Hide resolved
BALSAMIC/containers/multiqc/Dockerfile Outdated Show resolved Hide resolved
BALSAMIC/containers/multiqc/Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@ivadym ivadym left a comment

Choose a reason for hiding this comment

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

💯

BALSAMIC/assets/scripts/collect_qc_metrics.py Outdated Show resolved Hide resolved
BALSAMIC/assets/scripts/collect_qc_metrics.py Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
tests/scripts/test_collect_qc_metrics.py Outdated Show resolved Hide resolved
tests/scripts/test_collect_qc_metrics.py Outdated Show resolved Hide resolved
BALSAMIC/containers/multiqc/Dockerfile Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Jun 26, 2024

@mathiasbio
Copy link
Contributor Author

Thanks a lot for the review @ivadym ! ❤️

@mathiasbio
Copy link
Contributor Author

I will pull down the multiqc container to confirm it's still working, then squash and merge

@mathiasbio mathiasbio merged commit f173c27 into develop Jun 26, 2024
25 checks passed
@mathiasbio mathiasbio deleted the update_multiqc branch June 26, 2024 14:13
@mathiasbio mathiasbio mentioned this pull request Oct 17, 2024
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

Correct tool name in Sentieon Dedup metrics
2 participants