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

Qualimap: BamQC: add percentage on target #2020

Merged
merged 9 commits into from Sep 14, 2023
Merged

Conversation

FerriolCalvet
Copy link
Contributor

Solving this issue: #2019

  • This comment contains a description of changes (with reason)
    This is explained in the issue referenced above. But basically I am adding an extra column to the output table of Qualimap BAMQC. This value is already present in the default output of Qualimap BAMQC. It is the (97.14%).
    image

  • CHANGELOG.md has been updated

@vladsavelyev vladsavelyev self-requested a review August 31, 2023 15:51
Copy link
Member

@vladsavelyev vladsavelyev left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good to me.

I think it make sense to change the title of the existing regions_mapped_reads metric from "{} Aligned" to "{} On target", otherwise it has the same name as the overall number of mapped reads.

@vladsavelyev vladsavelyev added the waiting: response Waiting for more information from user label Sep 1, 2023
@vladsavelyev vladsavelyev self-requested a review September 1, 2023 09:48
Copy link
Member

@vladsavelyev vladsavelyev left a comment

Choose a reason for hiding this comment

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

Just rename the regions_mapped_reads metric from "{} Aligned" to "{} On target", please :)

@vladsavelyev vladsavelyev added awaits-review Awaiting final review and merge. and removed waiting: response Waiting for more information from user labels Sep 1, 2023
@vladsavelyev vladsavelyev changed the title qualimap bamqc: add percentage on target Qualimap BamQC: add percentage on target Sep 3, 2023
@vladsavelyev vladsavelyev changed the title Qualimap BamQC: add percentage on target Qualimap: BamQC: add percentage on target Sep 3, 2023
@vladsavelyev vladsavelyev added this to the MultiQC v1.16 milestone Sep 14, 2023
Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Looks good! Couple of minor pedantic comments, will fix quickly myself now.

multiqc/modules/qualimap/QM_BamQC.py Outdated Show resolved Hide resolved
multiqc/modules/qualimap/QM_BamQC.py Outdated Show resolved Hide resolved
multiqc/modules/qualimap/QM_BamQC.py Outdated Show resolved Hide resolved
@ewels
Copy link
Member

ewels commented Sep 14, 2023

I realised that several of my review comments also applied to existing code within the module! So was a good opportunity to clean those bits up as well ✅

Many thanks for this @FerriolCalvet!

@ewels ewels merged commit d601907 into MultiQC:master Sep 14, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits-review Awaiting final review and merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants