mosdepth missing values imputed

[adamrtalbot]

Changes:

• Mosdepth output has missing values in *.{region,global}.dist.txt
• This change to module fills any missing values with the next value
• e.g., if there is 100% at 100X and 80% at 80X, the value at 90X will be recorded as 80X
• Previously, this would have been recorded at 0%.
• This may underestimate coverage slightly but it's not clear from MosDepth docs how it should be handled.
• See .mosdepth.region.dist.txt missing values? brentp/mosdepth#190

---

• This comment contains a description of changes (with reason)
• CHANGELOG.md has been updated

[adamrtalbot]

Previous general stats table:

New general stats table:

[ewels]

Thanks! Speed thought: generally I try to avoid computing new data within MultiQC (plenty of exceptions abound). I agree that reporting 0.0% is just plain wrong, but what do you think about using None instead? (An empty table cell).

Won't be as pretty, but would be more true to the original data. This data is essentially the same as the plot lower in the report though, right? So prettiness shouldn't be super important? Or have I got that wrong?

[adamrtalbot]

Yours looks like that, but mine looks like this:

So perhaps we should fix at the source isntead? It's interpreting missing values as zero somewhere, we should stop this?

[ewels]

aha! Yes, if we remove the 0s so that they're not there (or are None, might work) then the plot will not have those points and the line will be smooth.

[adamrtalbot]

Oh hold on, different plots. I think the data in general stats is coming from my plot though.

[adamrtalbot]

I added some code where it evaluates every integer in the coverage values, but I haven't pushed it because while it parses the data fine and solves the problem, it takes forever to write the file.

Unfortunately using None instead of zero creates a coverage of None%.

[ewels]

Unfortunately using None instead of zero creates a coverage of None%.

Hmm, how annoying. Can we just remove the keys if None then please. Should work fine if they keys are just absent.

[adamrtalbot]

Before and after comparison:
multiqc_mosdepth_update.zip

[ewels]

Great! Just moved the log statement out of the loop as I was worried that there might be a lot per sample and it could make the debug log file huge.

Thanks for working on this and accepting feedback 👍🏻

[ewels]

Just before I merge - does this definitely fix the plot as well? The test data isn't affected like your screenshot above.. From a skim read of the code it now looks like it's only affecting values in the general stats table. Just want to check that the plot is good as well.

[adamrtalbot]

What was the problem again?

I think the main problem was the general stats table. The other problem is the coverage plot which isn’t fixed but I can look into that in the new year (in a new PR?)

[ewels]

Sorry, had a typo - I was talking about the plot. If you have some example data kicking around I can take a quick look for a fix.. I'd prefer to do both in one PR if possible, just so that we don't forget.

[ewels]

Ok, let's merge - I'm hoping to get a release out in 2022 and it would be good to include this 😅

[ewels]

I dropped the log message, hope that's ok. Figured that it's fairly self explanatory behaviour..

[adamrtalbot]

Thank you! I don’t have access to my laptop until next weekend so wouldn’t have been able to get this done for a while.

[ewels]

Very sensible! 👍🏻