Skip to content
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

add features into showMultiLinePlots.R and showMultiLinePlotsByVariable.R #86

Merged
merged 6 commits into from
Feb 8, 2024

Conversation

cchrisgong
Copy link
Contributor

  1. add options for multiple rows of subfigures for regional line plot in both
  2. for showMultiLinePlotsByVariable.R, add an option to show or not show global plot (to save space)

Copy link
Contributor

@pfuehrlich-pik pfuehrlich-pik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some code style remarks, would be great if you could fix all linter warnings.

@@ -27,6 +28,7 @@
#' @importFrom ggplot2 ylim
showMultiLinePlots <- function(
data, vars, scales = "free_y",
NROW_NUM = 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
NROW_NUM = 1,
nrowNum = 1,

This argument name would be more in line with our camelCase naming convention

@@ -92,39 +97,46 @@ showMultiLinePlotsByVariable <- function(

label <- paste0("(", paste0(levels(d$unit), collapse = ","), ")")
xLabel <- paste0(xVar, " (", paste0(levels(d$unit.x), collapse = ","), ")")


if (showGlobal) {
p1 <- dMainScen %>%
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please double check the indentation here, I think this needs to be indented more because you added the if

if (showHistorical) {
stopifnot(xVar %in% names(histRefModel))
if (showGlobal) {
p1 <- p1 +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check indentation

p2 <- p2 +
geom_point(data = dRegiHist, aes(shape = .data$model)) +
geom_line(data = dRegiHist, aes(group = paste0(.data$model, .data$region)), alpha = 0.5)
}
# Add markers for certain years.
if (length(yearsByVariable) > 0) {
if (showGlobal) {
p1 <- p1 +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check indentation

@@ -134,8 +146,10 @@ showMultiLinePlotsByVariable <- function(
}

# Show plots.
if (showGlobal) {
print(p1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check indentation

@cchrisgong cchrisgong merged commit e3cc350 into pik-piam:master Feb 8, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants