Updating default color palettes and adding custom_palette() #114

shaziaruybal · 2024-07-26T14:30:58Z

Pull Request

This PR responds to #20 and updates the default color palette to a hopefully nicer one from ggsci. It also fixes a bug (see 8a2d5bb and 193f172).

Additional changes:

A simple utility function custom_palette() was created to hopefully aid users to build their own custom palettes, especially if they want to visualise parameters with "multi-country" labels (eg "Guinea, Liberia, Sierra Leone") since the default palette will only work for single-country labels eg ("Guinea").
Tests for custom_palette() have also been added

Example usage of custom_palette():

p <- load_epidata("ebola")[["params"]]

# The default palette will only work for individual country labels, i.e. n=7 countries
forest_plot_incubation_period(p, col_by = "population_country")

# Use the custom_palette() to color all country labels, including 'multi-country' labels. In this example there are n=11 labels. 
labels <- c("DRC", "Guinea, Liberia, Sierra Leone", "Liberia, Nigeria", "Sudan", "DRC, Sudan", "Guinea, Sierra Leone", "Nigeria", "Uganda", "Guinea", "Liberia", "Sierra Leone")
colors <- c("indianred", "cornflowerblue", "forestgreen", "goldenrod", "magenta4", "turquoise4","black", "gold4", "skyblue1", "pink3", "darkgrey")

# Create the custom palette
custom_pal <- custom_palette(labels, colors)

# Plot with the custom_pal
forest_plot_incubation_period(p, col_by = "population_country",
                              col_palette = custom_pal)`

# Alternatively, use the function directly
forest_plot_incubation_period(p, col_by = "population_country",
                              col_palette = custom_palette(labels, colors))

Checklist:

[NA] I have added new dependencies in DESCRIPTION
[NA] I have bumped the version number in DESCRIPTION (I am now working off of an earlier version so didn't update any version numbers)
[NA] I have updated the NEWS.md
I have added unit tests using testthat
I have checked that the package can install locally using devtools::install_local("<path-to-epireview>")
I have added documentation where required
My documentation includes examples of how to use the function
[NA?] If necessary, I have updated priority-pathogens

…ot NA

…es ggsci color hex values

…stom palette

shaziaruybal · 2024-07-26T14:33:17Z

@tristan-myles just took a peek at the conflicts, I think the error message may need to also be updated for custom_palette() to be in line with cli_()

- Fixes conflicts in R/fig_utils, R/forest_plot, man/forest_plot

tristan-myles · 2024-08-10T14:01:33Z

R/forest_plot.R

@@ -90,11 +90,11 @@ forest_plot <- function(df, facet_by = NA, shape_by = NA, col_by = NA,
    ## use the palette if provided, otherwise use the default
    ## as defined in epireview
    ## if neither is provided, use the default palette
-    if (!is.na(shp_palette)) {
+    if (!is.null(shp_palette)) {
      p <- p + scale_shape_manual(values = shp_palette, na.value = 4)


@shaziaruybal Can you check that this na.value param is fine? I've added it based on the merge conflicts, but it wasn't in the initial version of develop that you were working on

@tristan-myles and I have checked and we think the na.value param is fine

tristan-myles · 2024-08-10T14:02:00Z

R/forest_plot.R

-    if (!is.na(col_palette)) {
-      p <- p + scale_color_manual(values = col_palette, na.value = "gray")
+    if (!is.null(col_palette)) {
+      p <- p + scale_color_manual(values = col_palette,  na.value = "gray")


@shaziaruybal Same as my previous comment, can you check that this na.value param is fine?

@tristan-myles and I have checked and we think the na.value param is fine

tristan-myles

In addition to my two comments on the files changed:

devtools::check is failing because load-filter-view-epidata.Rmd no longer compiles due to the changes in forest_plot.R. Could you check the functions used in this vignette. I think you just need to provide custom palettes?
Additionally, to confirm, is this the expected behaviour when now custom palette is provided?
I've bumped the version to 1.3.4 based on the anticipated merge of Sangeeta's PR, can you just add a brief description of the changes introduced in this PR in the NEWS.md file. A brief one-liner is fine. :)

- Fixes version conflict in DESCRIPTION

shaziaruybal · 2024-08-23T12:58:50Z

In addition to my two comments on the files changed:

devtools::check is failing because load-filter-view-epidata.Rmd no longer compiles due to the changes in forest_plot.R. Could you check the functions used in this vignette. I think you just need to provide custom palettes?
Additionally, to confirm, is this the expected behaviour when now custom palette is provided?

I've bumped the version to 1.3.4 based on the anticipated merge of Sangeeta's PR, can you just add a brief description of the changes introduced in this PR in the NEWS.md file. A brief one-liner is fine. :)

we fixed a minor bug when newest version of develop branch was merged and now builds - have added description to NEWS.md

- NA set to NULL

- Default param updated to NULL

tristan-myles

Passes tests and RCMD check passes

shaziaruybal added 6 commits June 6, 2024 15:45

fixed bug col_palette = NA and !is.na(col_palette) needs to be NULL n…

8a2d5bb

…ot NA

fixed bug shp_palette = NA and !is.na(shp_palette) needs to be NULL n…

193f172

…ot NA

update documentation

6db3fc4

update default color palette to ggsci and fix tests so that it expect…

410c1a6

…es ggsci color hex values

create new utility function custom_palette() for users to create a cu…

a55cc55

…stom palette

add tests for custom_palette()

db40927

shaziaruybal requested a review from tristan-myles July 26, 2024 14:30

tristan-myles added 2 commits August 10, 2024 14:40

Merge branch 'develop' into feature/forest_plot_colors

3700529

- Fixes conflicts in R/fig_utils, R/forest_plot, man/forest_plot

Bumps version number to 1.3.4

951ba76

tristan-myles reviewed Aug 10, 2024

View reviewed changes

tristan-myles requested changes Aug 10, 2024

View reviewed changes

Merge branch 'develop' into feature/forest_plot_colors

ff06250

- Fixes version conflict in DESCRIPTION

tristan-myles added 5 commits August 23, 2024 13:59

Updates parameter error introduced from merge

b35e4eb

- NA set to NULL

Merge branch 'develop' into feature/forest_plot_colors

35d69cd

Merge branch 'develop' into feature/forest_plot_colors

17fa211

Bumps vesion, adds news, & adds Shazia as author

fc856e8

Updates forest plot man

93faa5c

- Default param updated to NULL

tristan-myles approved these changes Aug 23, 2024

View reviewed changes

tristan-myles merged commit ef7105b into develop Aug 23, 2024
5 checks passed

tristan-myles deleted the feature/forest_plot_colors branch August 23, 2024 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating default color palettes and adding custom_palette() #114

Updating default color palettes and adding custom_palette() #114

shaziaruybal commented Jul 26, 2024

shaziaruybal commented Jul 26, 2024

tristan-myles Aug 10, 2024

shaziaruybal Aug 23, 2024

tristan-myles Aug 10, 2024

shaziaruybal Aug 23, 2024

tristan-myles left a comment •

edited

Loading

shaziaruybal commented Aug 23, 2024

tristan-myles left a comment

Updating default color palettes and adding custom_palette() #114

Updating default color palettes and adding custom_palette() #114

Conversation

shaziaruybal commented Jul 26, 2024

shaziaruybal commented Jul 26, 2024

tristan-myles Aug 10, 2024

Choose a reason for hiding this comment

shaziaruybal Aug 23, 2024

Choose a reason for hiding this comment

tristan-myles Aug 10, 2024

Choose a reason for hiding this comment

shaziaruybal Aug 23, 2024

Choose a reason for hiding this comment

tristan-myles left a comment • edited Loading

Choose a reason for hiding this comment

shaziaruybal commented Aug 23, 2024

tristan-myles left a comment

Choose a reason for hiding this comment

tristan-myles left a comment •

edited

Loading