morinlab / GAMBLR.viz

Collection of functions to make plots for Genomic Analysis of Mature B-cell Lymphomas in R
https://morinlab.github.io/GAMBLR.viz/
MIT License
0 stars 0 forks source link

Add internal call to prettyOncoplot to convert NA metadata values to literal "NA" string #42

Closed Kdreval closed 7 months ago

Kdreval commented 7 months ago

I think because when we don't supply the custom colors it is internally handled but when the user provides it then there is trust no modifications are necessary. We can probably add internal call to convert NAs to literal string anyways

Slack Message

Kdreval commented 7 months ago

In another use case, the issue with factor was also discovered. So when the metadata column is factor, ComplexHeatmap tries to map to numeric value of factor level, causing similar issue. We should add another internal call to convert metadata columns to characters if/when they are factors. So the fix should contain two steps: 1. convert from factor to string then 2. replace NA values with “NA”

rdmorin commented 7 months ago

I think there is a related issue here that should be resolved. Have those standard columns in the metadata (e.g. pathology) always been set as factors or is this a recent change?

Kdreval commented 7 months ago

I think I got the idea of what is going on

Kdreval commented 7 months ago

In the recent update that I made to get rid of colors in the annotation colors that are displayed when no values for them are present in the metadata (PR #36, issue #35) I did not account for the metadata columns that had been passed to the prettyOncoplot as factors. This resulted in a bug that during subsetting of the colors list, the NA values were not properly handled, which generated this issue. There was already a call that was converting factors to the characters, but it was done during subsetting for the remaining colors, and to fix the reported issue I have moved this call to occur prior to the color subsetting. Now the example works properly and can handle the sex column that is passed as factor. Here is the sample plot: image

Kdreval commented 7 months ago

This is fixed in PR #44