search

DIFM-Brain / ofpetrial

GNU General Public License v3.0
0 stars 1 forks source link

remove dependency to RColorBrewer #15

Closed tmieno2 closed 4 months ago

tmieno2 commented 4 months ago

@brittanikedge, can you remove dependency to the RColorBrewer package? If I am not mistaken, the only used function from this package is brewer.pal() (I don't think you are using any of the palettes from the pacakge). You can create a data where n_rates is one variable and a vector of color sequences is another. You can then look up the data based on n_rates.

brittanikedge commented 4 months ago

@tmieno2 Sure, I don't think I am either, but I will check this in the report and make the necessary adjustments.

tmieno2 commented 4 months ago

@brittanikedge, is there a reason we have + 3 in below?

RColorBrewer::brewer.pal(n = n_rates + 3, "Greys")[1:n_rates]

I think it is very unlikely, but when n_rates is greater than equal to 7, it produces an error saying n too large.

brittanikedge commented 4 months ago

@tmieno2 I'm looking at the code now, and I don't see why the + 3 is necessary. I can remove it and test a few examples to see if it works.

brittanikedge commented 4 months ago

@tmieno2 After looking at where this code was used in the maps, we don't need to have colors for all of the rate levels in the palette. This is used in the case where one of the input plots is larger than the other, so for one of the inputs we will have two adjacent plots in the machinery figures. Given that it is highly unlikely that one plot size will be more than two or three times the size of the other, we only need two or three colors in the palette.

Rather than designing the palette with all of the possible input rates, I changed the code to consider the colors that are present in the plots that are going to be graphed. I tested it out, and it looks good. We aren't putting a legend in the figure to say what rate the colors represent. It's just to distinguish the two plots from one another.