Closed himmil closed 8 months ago
Tests (tests/testthat directory) seems still have deprecated functions
Related to what we discussed today; to replace aes_string
:
There are two cases where we have used aes_string
: "the standard way", i.e. case 1, and with do.call
function, i.e., case 3
Below you will find, how to replace these
library(ggplot2)
data(cars)
######################### CASE 1
p <- ggplot(cars)
p + geom_point(aes(x = speed, y = dist))
p + geom_point(aes_string(x = "speed", y = "dist"))
p + geom_point(aes(x = .data[["speed"]], y = .data[["dist"]]))
######################### CASE2
p + aes(x = speed, y = dist) + geom_point()
p + aes_string(x = "speed", y = "dist") + geom_point()
p + aes(x = .data[["speed"]], y = .data[["dist"]]) + geom_point()
############################ CASE 2.1
aesthetic <- aes(x = speed, y = dist)
p + aesthetic + geom_point()
aesthetic <- aes_string(x = "speed", y = "dist")
p + aesthetic + geom_point()
aesthetic <- aes(x = .data[["speed"]], y = .data[["dist"]])
p + aesthetic + geom_point()
################# CASE 3
# IS not working
args <- list(x = "speed", y = "dist")
aesthetic <- do.call(aes, args)
p + aesthetic + geom_point()
# Works
args <- list(x = "speed", y = "dist")
aesthetic <- do.call(aes_string, args)
p + aesthetic + geom_point()
# Option 1 (I think this is better)
args <- list(x = "speed", y = "dist")
args <- lapply(args, function(x) if (!is.null(x)) sym(x))
aesthetic <- do.call(aes, args)
p + aesthetic + geom_point()
# Option 2
args <- list(x = "speed", y = "dist")
args <- lapply(args, function(x) if (!is.null(x)) sym(x))
aesthetic <- aes(!!!args)
p + aesthetic + geom_point()
Error occurs when the data is loaded
no visible global function --> add importFrom tags (check other functions for example)
Now, finally, it should be ready to be merged.
Sorry for late reply. Can you run BiocCheck::BiocCheck(). It seems that the indentation is off (should be multiplication of 4 spaces)
Hi, I ran the check and it gave an error that installation calls found in vignettes. How should I fix it?
You can skip those errors not directly related to this PR. We can fix those things in other PRs
Ok, this is ready to be merged (I don't have write access to do it).
@TuomasBorman can you merge?