-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correct PlotFertilityRatesStats() to use new rates format #83
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested plots option, would suggest adding some docs to describe how SE is used in display the interval of plots
@@ -43,6 +36,7 @@ | |||
}) | |||
|
|||
df <- data.table::rbindlist(l) | |||
df <- subset(df, !startsWith(df$Label, "NA")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we purposely leave the label blank for zero-fertility group from the PopValues ? it'd be nice to show these zero groups as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ... good question. Several thoughts:
- In the special case of fertility, it's mostly accepted that fertility is zero before a certain age and after a certain ago, so plotting those regions is probably not informative. (If the fertility rate of 80-year-old women isn't exactly zero, it's so low as not to affect the population predictions.)
- The NAs happen when a label isn't provided, so the simple workaround for missing rate bands is to provide a label. However, this leads to the next problem ...
- The log of zero is -Inf, so these regions won't show up in the log plot anyway. And elsewhere in the code we assume that zero-rate stuff is intended to stay zero, so the non-log plot will be a perfect straight line.
I'm open to suggestions for a different approach. If you have a suggestion, post a feature request and we can discuss.
@@ -58,7 +52,9 @@ | |||
#' Plot Fertility Rates Statistics | |||
#' | |||
#' @param results Results list (as returned by \code{RunExperiments()}) | |||
#' @param se Whether to show standard error intervals, or projection intervals | |||
#' @param se Whether to show standard error or standard deviation confidence intervals | |||
#' @param type Plot type (options = "lines", "ribbon", "boxplot") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the boxplot does not seem to use standard error option, suggest document this
if (log){ | ||
g <- ggplot(df, aes( | ||
x = Year, | ||
y = LogRate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this work with y = ifelse(log, LogRate, Rate)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. As you know, in ggplot column references don't have quotation marks around them, and that implies some tricky parse/evaluate logic inside ggplot. I'm concerned that if we took the ifelse approach, ggplot would go looking for a column called "ifelse(log, LogRate, Rate)". I haven't confirmed this.
Fixes #79