Added intuitive colors and color option #173
Conversation
1. I added more intuitive colors to identify the state of the colors to avoid any kind of MISLEADING interpretation. Ex: "Red = remove" is more intuitive to know that column needs to be removed than the default ggplot colors (blue or pink). 2. I added a color option if anyone wants to use other colors (which might be more intuitive for them)
|
My pull also solves the inconsistency problem mentioned here: #129 |
| @@ -26,6 +26,10 @@ | |||
|
|
|||
| plot_missing <- function(data, | |||
| group = list("Good" = 0.05, "OK" = 0.4, "Bad" = 0.8, "Remove" = 1), | |||
| fill_color = c("Good" = "#1B9E77", | |||
There was a problem hiding this comment.
This will work for the default case, but will break if number of groups is not 4.
There was a problem hiding this comment.
Good point! I think I only need to add error messages when groups' names are different from fill_color's names to make them aware of their errors, most people will use the default groups.
1. I added more intuitive colors to identify the state of the colors to avoid any kind of MISLEADING interpretation. Ex: "Red = remove" is more intuitive to know that column needs to be removed than the default ggplot colors (blue or pink). 2. Added a color option if anyone wants to use other colors (which might be more intuitive for them) 3. Add error messages when groups' names are different from fill_color's names
1. I added more intuitive colors to identify the state of the colors to avoid any kind of MISLEADING interpretation. Ex: "Red = remove" is more intuitive to know that column needs to be removed than the default ggplot colors (blue or pink). 2. Added a color option if anyone wants to use other colors (which might be more intuitive for them) 3. Add error messages when groups' names are different from fill_color's names
R/plot_missing.r
Outdated
| missing_only = FALSE, | ||
| geom_label_args = list(), | ||
| title = NULL, | ||
| ggtheme = theme_gray(), | ||
| theme_config = list("legend.position" = c("bottom"))) { | ||
| ## Verify group = fill_color | ||
| if (length(group) != length(fill_color)) { |
There was a problem hiding this comment.
I have mixed feelings about these. This is what I think the update should include:
- When using default, I am fine applying a hard-coded color template.
- When not using default (not 4 groups), the color template should be automated instead of throwing an error.
- End user should be able to define a customized color template.
I believe we have 1 and 3, but might need to tweak for 2.
1. When using the default, applies a hard-coded color template. 2. When not using the default (not 4 groups), the color template uses ggplot's default colors. 3. End user is able to define a customized color template.
Just change color_fill to plot_fill to make the code easier to understand
|
Hi @davidr9708 , would you mind pulling your PR into the latest master, since there are some updates in the same file. Apologies for the inconvenience. |
|
Don't worry, I just pulled it with the new updates of the master branch, please, I'm sure there's not any problem but please, test it before merging it |
|
Thank you so much @davidr9708 ! I will merge within a few days. Having a busy week and thanks for your patience! |
|
Duplicate of #177 |
I added more intuitive colors to identify the state of the colors to avoid any kind of MISLEADING interpretation. Ex: "Red = remove" is more intuitive to know that column needs to be removed than the default ggplot colors (blue or pink).
I added a color option if anyone wants to use other colors (which might be more intuitive for them)