CN Status Heatmap (PR 2 of 2) #603
Conversation
cansavvy
added
the
work in progress
cansavvy
requested review from
jashapiro and
cbethell
and removed request for
jashapiro
There was a problem hiding this comment.
Hi @cansavvy, this looks just about ready to merge to me!
I did however notice that the bottom CN status annotation bar on the final heatmap does not appear to reflect all of the calls (it appears to reflect only losses and gains although the legend also includes neutral, unstable and uncallable).
That being said, is the bottom annotation based on the majority call for a whole chromosome? If so, would it be possible to add the chromosome labels to make it clear that the bottom annotation bar is relevant to each individual chromosome (although I am sure that this will be included in the figure description).
I am going to approve this PR because I believe it can be merged without said labels 👍 although they would be nice.
I'm not seeing this? When I'm looking at the plot it looks like all of the labels are in the legend? Sometimes the Markdown document cuts off part of it in the preview, but if you look at the pdf, it looks fine. |
The legend looks fine, I am referring to the bottom annotation red and blue bar. That bottom bar reflects the chromosomal majority CN status, is that correct? |
| frac_loss > threshold ~ "loss", | ||
| frac_neutral > threshold ~ "neutral", | ||
| TRUE ~ "unstable" | ||
| frac_uncallable > frac_uncallable_val ~ "uncallable", |
There was a problem hiding this comment.
Is this what we wanted here? Or should we keep what's in master?
There was a problem hiding this comment.
It looks like this is correct, as it now matches the function arguments. However, the docs for the function should also be changed to match.
There was a problem hiding this comment.
Unless you were looking at the order of the final two options, but I think that what you have is correct, because you got strange results the other way? I don't fully remember.
There was a problem hiding this comment.
Yes. I think we had settled on this. https://alexslemonade.slack.com/archives/CNH4FND1C/p1586786889017900
Will make sure docs are updated though.
I think what you are looking at is just the visual cue for where each chromosome starts and ends. Perhaps the color should be changed to avoid confusion with calls though. |
| seg_data <- data.table::fread(file.path( | ||
| input_dir, | ||
| "pbta-cnv-consensus.seg.gz" | ||
| ), | ||
| data.table = FALSE | ||
| ) |
There was a problem hiding this comment.
Indentation here is strange.
| seg_data <- data.table::fread(file.path( | |
| input_dir, | |
| "pbta-cnv-consensus.seg.gz" | |
| ), | |
| data.table = FALSE | |
| ) | |
| seg_data <- data.table::fread( | |
| file.path( | |
| input_dir, | |
| "pbta-cnv-consensus.seg.gz" | |
| ), | |
| data.table = FALSE | |
| ) |
There was a problem hiding this comment.
I ran styler on it. It doesn't always make perfect choices.
| seg_data <- seg_data %>% | ||
| # Join the histology column to this data | ||
| dplyr::inner_join(dplyr::select( | ||
| metadata, | ||
| "Kids_First_Biospecimen_ID", | ||
| "short_histology", | ||
| "tumor_ploidy" | ||
| ), | ||
| by = c("ID" = "Kids_First_Biospecimen_ID") |
There was a problem hiding this comment.
Same thing... try not to open 2 sets of parens on the same line that don't then close together, as it makes for non-semantic indentation.
| seg_data <- seg_data %>% | |
| # Join the histology column to this data | |
| dplyr::inner_join(dplyr::select( | |
| metadata, | |
| "Kids_First_Biospecimen_ID", | |
| "short_histology", | |
| "tumor_ploidy" | |
| ), | |
| by = c("ID" = "Kids_First_Biospecimen_ID") | |
| seg_data <- seg_data %>% | |
| # Join the histology column to this data | |
| dplyr::inner_join( | |
| dplyr::select( | |
| metadata, | |
| "Kids_First_Biospecimen_ID", | |
| "short_histology", | |
| "tumor_ploidy" | |
| ), | |
| by = c("ID" = "Kids_First_Biospecimen_ID") |
| show_row_names = FALSE, | ||
| bottom_annotation = chr_annot, | ||
| right_annotation = hist_annot, | ||
| heatmap_legend_param = list(nrow = 1) |
There was a problem hiding this comment.
The heatmap itself is drawn rasterized, which is mostly fine, but may be a bit lower resolution than ideal. Can we see what happens if we bump it up, both to file size and image quality? You might even go higher than this, depending on results. It is a bit hard to interpret the docs https://jokergoo.github.io/ComplexHeatmap-reference/book/a-single-heatmap.html#heatmap-as-raster-image, but if the general setting is equivalent to 72dpi, we might want to go as far as 4 here.
| heatmap_legend_param = list(nrow = 1) | |
| heatmap_legend_param = list(nrow = 1), | |
| raster_quality = 2 |
This reverts commit 9bc1a10.
Purpose/implementation Section
What scientific question is your analysis addressing?
We wanted a summary visualization of copy number status.
This second PR has the main notebook where the functions from the previous PR are implemented.
What was your approach?
Create a summary heatmap of copy number status from the consensus CNV call data.
This is done by binning the genome and calculating the segment's coverage of the
CNV consensus segments.
A bin is declared a particular copy number status if that status's base pair
coverage is a certain threshold percentage larger than the other statuses'
coverage.
What GitHub issue does your pull request address?
Issue: #594
Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.
Which areas should receive a particularly close look?
Results
See the rendered notebook here:
https://cansavvy.github.io/openpbta-notebook-concept/cnv-chrom-plot/cn_status_heatmap.nb.html
Reproducibility Checklist
Documentation Checklist
**These items already existed but have been updated in #602
READMEand it is up to date.analyses/README.mdand the entry is up to date.