init prettyCoOncoplot #92
Conversation
R/viz.R
Outdated
| #' ssm1=maftools::read.maf(ssm1) | ||
| #' ssm2=get_coding_ssm(limit_cohort = "BL_Pediatric") | ||
| #' ssm2=maftools::read.maf(ssm2) | ||
| #' meta1=get_gambl_metadata() %>% filter(cohort=="BL_Adult") |
There was a problem hiding this comment.
Not sure how important this is, but when I tried to run the example I got an error saying "Error in filter(., cohort == "BL_Pediatric") : object 'cohort' not found ". This is since the package related to the intended filter function is not specified. Adding dplyr:: in front of the filter statement solved it and the example runs. Just a minor comment. The rest of the example works great and I now have a side-by-side (very) Pretty Oncoplot :)
There was a problem hiding this comment.
Yes, this happens for me too when I run devtools::load_all(). You are right, the example should say dplyr::filter - I will correct this and re-generate documentation
| } | ||
|
|
||
| # Arguments to pass into prettyOncoplot | ||
| oncoplot_args = list(...) |
| @@ -1725,11 +1725,11 @@ get_gambl_colours = function(classification = "all", | |||
| "NEG" = "#E5A4CB") | |||
|
|
|||
| all_colours[["BL"]] = c("M53-BL" = "#A6CEE3", | |||
| "DLBCL-1" = "#721F0F", | |||
| "DLBCL-A" = "#721F0F", | |||
There was a problem hiding this comment.
Just curious, what's the rationale for moving away from numbers to letters in this case?
There was a problem hiding this comment.
Ah, this is because we will rename these subgroups as part of our response to reviewer's comments fr our manuscript. The concern is that the clusters C1 to C5 already exist for DLBCL, and this may create confusion
| @@ -1093,8 +1093,10 @@ get_cn_states = function(regions_list, | |||
| cn_matrix = pivot_wider(all_cn, id_cols = "sample_id", names_from = "region_name", values_from = "CN") %>% | |||
| column_to_rownames("sample_id") | |||
|
|
|||
| names(cn_matrix) = region_names | |||
There was a problem hiding this comment.
Why was this changed? Is it a bug fix? It seems like it will affect the output.
There was a problem hiding this comment.
Yes, I had this not working as it was before. I think it might be related to R v3.6 to R v4 shift. Basically it was returning an unnamed vector of numbers (CN states) instead of a named column in a df
| #' fontSizeGene=12, | ||
| #' metadataBarFontsize=10, | ||
| #' label1="Adult", | ||
| #' label2="Pediatric") |
There was a problem hiding this comment.
I have a suggestion for how to make this function easier to use. What about implementing it the same way prettyForestPlot is set up. The user doesn't have to worry about subsetting. They just provide one mutation_df and one metadata table and specify the column and two values to use to subset the data to the two groups. Would this have any drawbacks?
There was a problem hiding this comment.
Totally possible to do, sure! Should be easy to implement. I agree it will make the life of a user better!
|
Thank you both for review and approval! |
Adding the functionality to show 2 prettyOncoplots side-by-side. The output is ggplot-compatible and can be arranged together with other plots in a multi-panel figure.