Skip to content

Commit e6cc271

Browse files
authored
Use faster versions of system.file()/packageVersion()/is_installed() (#2072)
1 parent 3d49fbc commit e6cc271

8 files changed

+123
-22
lines changed

R/ggplotly.R

+2-2
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ gg2list <- function(p, width = NULL, height = NULL,
181181
grDevices::png
182182
} else if (capabilities("jpeg")) {
183183
grDevices::jpeg
184-
} else if (system.file(package = "Cairo") != "") {
184+
} else if (is_installed("Cairo")) {
185185
function(filename, ...) Cairo::Cairo(file = filename, ...)
186186
} else {
187187
stop(
@@ -243,7 +243,7 @@ gg2list <- function(p, width = NULL, height = NULL,
243243
# currently, LayerSf is the only core-ggplot2 Layer that makes use
244244
# of it https://github.com/tidyverse/ggplot2/pull/2875#issuecomment-438708426
245245
data <- layer_data
246-
if (packageVersion("ggplot2") > "3.1.0") {
246+
if (get_package_version("ggplot2") > "3.1.0") {
247247
data <- by_layer(function(l, d) if (is.function(l$setup_layer)) l$setup_layer(d, plot) else d)
248248
}
249249

R/plotly.R

+3-3
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ plot_geo <- function(data = data.frame(), ..., offline = FALSE) {
274274
p <- plot_ly(data, ...)
275275

276276
if (isTRUE(offline)) {
277-
if (system.file(package = "plotlyGeoAssets") == "") {
277+
if (!is_installed("plotlyGeoAssets")) {
278278
stop(
279279
"The plotlyGeoAssets package is required to make 'offline' maps. ",
280280
"Please install and try again.",
@@ -491,7 +491,7 @@ plotlyMainBundlePath <- function() {
491491
dep <- plotlyMainBundle()
492492
path <- file.path(dep$src$file, dep$script)
493493
if (!is.null(dep$package)) {
494-
path <- system.file(path, package = dep$package)
494+
path <- system_file(path, package = dep$package)
495495
}
496496
path
497497
}
@@ -513,7 +513,7 @@ locale_dependency <- function(locale) {
513513
}
514514

515515
locale_dir <- dependency_dir("plotlyjs", "locales")
516-
locales_all <- sub("\\.js$", "", list.files(system.file(locale_dir, package = "plotly")))
516+
locales_all <- sub("\\.js$", "", list.files(system_file(locale_dir, package = "plotly")))
517517
if (!tolower(locale) %in% locales_all) {
518518
stop(
519519
"Invalid locale: '", locale, "'.\n\n",

R/plotly_example.R

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ plotly_example <- function(type = c("demo", "shiny", "rmd"), name, edit = TRUE,
2424
}
2525

2626
# check to make sure the example exists
27-
exampleDir <- system.file("examples", type, package = "plotly")
27+
exampleDir <- system_file("examples", type, package = "plotly")
2828
nms <- basename(list.dirs(exampleDir, recursive = FALSE))
2929
if (missing(name) || !isTRUE(name %in% nms)) {
3030
message(
@@ -36,7 +36,7 @@ plotly_example <- function(type = c("demo", "shiny", "rmd"), name, edit = TRUE,
3636
return(invisible())
3737
}
3838

39-
finalDir <- system.file("examples", type, name, package = "plotly")
39+
finalDir <- system_file("examples", type, name, package = "plotly")
4040
if (edit) {
4141
files <- list.files(finalDir, full.names = TRUE)
4242
scripts <- files[tools::file_ext(files) %in% c("R", "Rmd")]

R/shiny.R

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ plotlyOutput <- function(outputId, width = "100%", height = "400px",
3434
package = "plotly",
3535
reportSize = TRUE
3636
)
37-
if (is_available("shiny", "1.4.0.9003") && is_available("htmlwidgets", "1.5.2.9000")) {
37+
if (is_installed("shiny", "1.4.0.9003") && is_installed("htmlwidgets", "1.5.2.9000")) {
3838
args$reportTheme <- reportTheme
3939
}
4040
do.call(htmlwidgets::shinyWidgetOutput, args)

R/signup.R

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ signup <- function(username, email, save = TRUE) {
4242
un = username,
4343
email = email,
4444
platform = "R",
45-
version = as.character(packageVersion("plotly"))
45+
version = as.character(get_package_version("plotly"))
4646
)
4747
base_url <- file.path(get_domain(), "apimkacct")
4848
resp <- httr::RETRY(

R/staticimports.R

+107
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Generated by staticimports; do not edit by hand.
2+
# ======================================================================
3+
# Imported from pkg:staticimports
4+
# ======================================================================
5+
6+
# Borrowed from pkgload:::dev_meta, with some modifications.
7+
# Returns TRUE if `pkg` was loaded with `devtools::load_all()`.
8+
devtools_loaded <- function(pkg) {
9+
ns <- .getNamespace(pkg)
10+
if (is.null(ns) || is.null(ns$.__DEVTOOLS__)) {
11+
return(FALSE)
12+
}
13+
TRUE
14+
}
15+
16+
get_package_version <- function(pkg) {
17+
# `utils::packageVersion()` can be slow, so first try the fast path of
18+
# checking if the package is already loaded.
19+
ns <- .getNamespace(pkg)
20+
if (is.null(ns)) {
21+
utils::packageVersion(pkg)
22+
} else {
23+
as.package_version(ns$.__NAMESPACE__.$spec[["version"]])
24+
}
25+
}
26+
27+
is_installed <- function(pkg, version = NULL) {
28+
installed <- isNamespaceLoaded(pkg) || nzchar(system_file_cached(package = pkg))
29+
if (is.null(version)) {
30+
return(installed)
31+
}
32+
installed && isTRUE(get_package_version(pkg) >= version)
33+
}
34+
35+
# Borrowed from pkgload::shim_system.file, with some modifications. This behaves
36+
# like `system.file()`, except that (1) for packages loaded with
37+
# `devtools::load_all()`, it will return the path to files in the package's
38+
# inst/ directory, and (2) for other packages, the directory lookup is cached.
39+
# Also, to keep the implementation simple, it doesn't support specification of
40+
# lib.loc or mustWork.
41+
system_file <- function(..., package = "base") {
42+
if (!devtools_loaded(package)) {
43+
return(system_file_cached(..., package = package))
44+
}
45+
46+
if (!is.null(names(list(...)))) {
47+
stop("All arguments other than `package` must be unnamed.")
48+
}
49+
50+
# If package was loaded with devtools (the package loaded with load_all),
51+
# also search for files under inst/, and don't cache the results (it seems
52+
# more likely that the package path will change during the development
53+
# process)
54+
pkg_path <- find.package(package)
55+
56+
# First look in inst/
57+
files_inst <- file.path(pkg_path, "inst", ...)
58+
present_inst <- file.exists(files_inst)
59+
60+
# For any files that weren't present in inst/, look in the base path
61+
files_top <- file.path(pkg_path, ...)
62+
present_top <- file.exists(files_top)
63+
64+
# Merge them together. Here are the different possible conditions, and the
65+
# desired result. NULL means to drop that element from the result.
66+
#
67+
# files_inst: /inst/A /inst/B /inst/C /inst/D
68+
# present_inst: T T F F
69+
# files_top: /A /B /C /D
70+
# present_top: T F T F
71+
# result: /inst/A /inst/B /C NULL
72+
#
73+
files <- files_top
74+
files[present_inst] <- files_inst[present_inst]
75+
# Drop cases where not present in either location
76+
files <- files[present_inst | present_top]
77+
if (length(files) == 0) {
78+
return("")
79+
}
80+
# Make sure backslashes are replaced with slashes on Windows
81+
normalizePath(files, winslash = "/")
82+
}
83+
84+
# A wrapper for `system.file()`, which caches the results, because
85+
# `system.file()` can be slow. Note that because of caching, if
86+
# `system_file_cached()` is called on a package that isn't installed, then the
87+
# package is installed, and then `system_file_cached()` is called again, it will
88+
# still return "".
89+
system_file_cached <- local({
90+
pkg_dir_cache <- character()
91+
92+
function(..., package = "base") {
93+
if (!is.null(names(list(...)))) {
94+
stop("All arguments other than `package` must be unnamed.")
95+
}
96+
97+
not_cached <- is.na(match(package, names(pkg_dir_cache)))
98+
if (not_cached) {
99+
pkg_dir <- system.file(package = package)
100+
pkg_dir_cache[[package]] <<- pkg_dir
101+
} else {
102+
pkg_dir <- pkg_dir_cache[[package]]
103+
}
104+
105+
file.path(pkg_dir, ...)
106+
}
107+
})

R/utils.R

+6-12
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# @staticimports pkg:staticimports
2+
# is_installed get_package_version system_file
3+
14
is.plotly <- function(x) {
25
inherits(x, "plotly")
36
}
@@ -1089,7 +1092,7 @@ get_kwargs <- function() {
10891092

10901093
# "common" POST header fields
10911094
api_headers <- function() {
1092-
v <- as.character(packageVersion("plotly"))
1095+
v <- as.character(get_package_version("plotly"))
10931096
httr::add_headers(
10941097
plotly_version = v,
10951098
`Plotly-Client-Platform` = paste("R", v),
@@ -1130,22 +1133,13 @@ cat_profile <- function(key, value, path = "~") {
11301133

11311134
# check that suggested packages are installed
11321135
try_library <- function(pkg, fun = NULL) {
1133-
if (system.file(package = pkg) != "") {
1136+
if (is_installed(pkg)) {
11341137
return(invisible())
11351138
}
11361139
stop("Package `", pkg, "` required", if (!is.null(fun)) paste0(" for `", fun, "`"), ".\n",
11371140
"Please install and try again.", call. = FALSE)
11381141
}
11391142

1140-
# a la shiny:::is_available
1141-
is_available <- function(package, version = NULL) {
1142-
installed <- nzchar(system.file(package = package))
1143-
if (is.null(version)) {
1144-
return(installed)
1145-
}
1146-
installed && isTRUE(utils::packageVersion(package) >= version)
1147-
}
1148-
11491143
# similar logic to rstudioapi::isAvailable()
11501144
is_rstudio <- function() {
11511145
identical(.Platform$GUI, "RStudio")
@@ -1168,7 +1162,7 @@ longest_element <- function(x) {
11681162

11691163
# A dplyr::group_by wrapper for the add argument
11701164
group_by_add <- function(..., add = TRUE) {
1171-
if (packageVersion('dplyr') >= '1.0') {
1165+
if (get_package_version('dplyr') >= '1.0') {
11721166
dplyr::group_by(..., .add = add)
11731167
} else {
11741168
dplyr::group_by(..., add = add)

tests/testthat/test-plotly-color.R

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ test_that("Custom RColorBrewer pallette works for factor variable", {
3232
l <- expect_traces(p, 3, "scatterplot-color-factor-custom")
3333
markers <- lapply(l$data, "[[", "marker")
3434
colz <- unlist(lapply(markers, "[[", "color"))
35-
idx <- if (packageVersion("scales") > '1.0.0') c(1, 2, 3) else c(1, 5, 9)
35+
idx <- if (get_package_version("scales") > '1.0.0') c(1, 2, 3) else c(1, 5, 9)
3636
expect_identical(sort(colsToCompare[idx]), sort(colz))
3737
# providing vector of RGB codes should also work
3838
p <- plot_ly(d, x = ~bill_length_mm, y = ~flipper_length_mm, color = ~species,

0 commit comments

Comments
 (0)