From fbe1aa0e36df289b27881d077635352e6debdbc1 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 17:28:46 -0600 Subject: [PATCH 01/21] Started packages_from_github --- DESCRIPTION | 2 +- NEWS.md | 4 ++++ R/load_package.r | 24 +++++++++++++++++++----- R/packages.r | 4 ++-- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 8939359..dde5273 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -4,7 +4,7 @@ Description: A "define" function is provided that allows for inclusion of external scripts without polluting the global namespace and makes it easier to write modular R code. -Version: 0.2.2 +Version: 0.3 Author: Robert Krzyzanowski Maintainer: Robert Krzyzanowski Depends: diff --git a/NEWS.md b/NEWS.md index 638d3c7..0e9a41c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# Version 0.3 + + * `packages` will now install from GitHub when the package name contains a '/'. + # Version 0.2.1 * The `define` function takes an `envir` argument that can be used diff --git a/R/load_package.r b/R/load_package.r index 417c016..0fa6f10 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -4,12 +4,26 @@ #' @param name Name of package #' @examples #' \dontrun{ -#' load_package('glmnet') +#" load_package("glmnet") #' } load_package <- function(name) { - if (!require(name, character.only = TRUE)) { - install.packages(name, dependencies = TRUE) - if (!require(name, character.only = TRUE)) - stop(paste('Package', name, 'not found')) + if (package_is_installed(name)) { return(TRUE) } + if (is_github_package(name)) { + remote <- "github" + devtools::install_github(package) + } else { + remote <- "CRAN" + install.packages(package) # install from CRAN } + if (!package_is_installed(name)) { + stop(paste('Package', name, "not found on", remote, ".")) + } +} + +package_is_installed <- function(name) { + require(name, character.only = TRUE) +} + +is_github_package <- function(name) { + grepl("/", name, fixed = TRUE) } diff --git a/R/packages.r b/R/packages.r index 1fd7a81..a0d391d 100644 --- a/R/packages.r +++ b/R/packages.r @@ -5,8 +5,8 @@ #' @examples #' \dontrun{ #' packages('glmnet', 'caret') -#' packages('glmnet caret') -#' packages(list('glmnet', 'caret'), c('e1071', 'parallel multicore'), 'stringr') +#' packages('glmnet caret') # Can just separate with a space +#' packages(list('glmnet', 'caret'), c('e1071', 'parallel multicore'), 'stringr') # can nest space and vectorization #' } packages <- function(...) { split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_]+')[[1]] From 363881999b68e5006eab1fba8f15111a4c2420b2 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 17:38:15 -0600 Subject: [PATCH 02/21] Add verbose option; fix bugs --- NEWS.md | 1 + R/load_package.r | 23 ++++++++++++++++++----- R/packages.r | 19 +++++++++++++------ 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0e9a41c..8a731e7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,7 @@ # Version 0.3 * `packages` will now install from GitHub when the package name contains a '/'. + * `packages` will now take `verbose = TRUE` to announce the installations. # Version 0.2.1 diff --git a/R/load_package.r b/R/load_package.r index 0fa6f10..ce37a54 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -1,26 +1,39 @@ #' Load a package and install it if necessary #' #' @name load_package -#' @param name Name of package +#' @param name character. Name of package. +#' @param verbose logical. Whether or not to announce each installation. #' @examples #' \dontrun{ #" load_package("glmnet") #' } -load_package <- function(name) { - if (package_is_installed(name)) { return(TRUE) } +load_package <- function(name, verbose = FALSE) { + if (package_is_installed(name)) { + if (isTRUE(verbose)) { message(name, " already installed.") } + return(TRUE) + } if (is_github_package(name)) { remote <- "github" - devtools::install_github(package) + if (isTRUE(verbose)) { announce(name, remote) } + devtools::install_github(name) } else { remote <- "CRAN" - install.packages(package) # install from CRAN + if (isTRUE(verbose)) { announce(name, remote) } + install.packages(name) # install from CRAN } if (!package_is_installed(name)) { stop(paste('Package', name, "not found on", remote, ".")) } } +announce <- function(name, remote) { + message("Installing ", name, " from ", remote, ".") +} + package_is_installed <- function(name) { + if (is_github_package(name)) { + name <- strsplit("robertzk/Ramd", "/")[[1]][[2]] + } require(name, character.only = TRUE) } diff --git a/R/packages.r b/R/packages.r index a0d391d..d7fcdde 100644 --- a/R/packages.r +++ b/R/packages.r @@ -1,16 +1,23 @@ #' Load a bunch of packages #' #' @param ... List of packages +#' @param verbose logical. Whether or not to announce installations and +#' package startup messages. #' @export #' @examples #' \dontrun{ -#' packages('glmnet', 'caret') -#' packages('glmnet caret') # Can just separate with a space -#' packages(list('glmnet', 'caret'), c('e1071', 'parallel multicore'), 'stringr') # can nest space and vectorization +#' packages("glmnet", "caret") +#' packages("glmnet caret") # Can just separate with a space +#' packages(list("glmnet", "caret"), c("e1071", "parallel multicore"), "stringr") # can nest space and vectorization +#' packages("robertzk/Ramd") # Can install from GitHub +#' packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") +#' packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub #' } -packages <- function(...) { - split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_]+')[[1]] +#' @return TRUE +packages <- function(..., verbose = FALSE) { + split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/]+')[[1]] pkgs <- unlist(lapply(unlist(list(...)), split_packages)) - suppressPackageStartupMessages(sapply(pkgs, load_package)) + if (isTRUE(verbose)) { sapply(pkgs, load_package, verbose = TRUE) } + else { suppressPackageStartupMessages(sapply(pkgs, load_package)) } TRUE } From 4d85e5fd75f4511016a6bf71c84a09bb41e78ea1 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 17:39:09 -0600 Subject: [PATCH 03/21] Redoc --- NAMESPACE | 2 +- man/Ramd.Rd | 2 +- man/define.Rd | 2 +- man/load_dependency.Rd | 2 +- man/load_package.Rd | 9 +++++---- man/packages.Rd | 19 ++++++++++++++----- man/pp.Rd | 2 +- 7 files changed, 24 insertions(+), 14 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index b3d8518..5ceda64 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,4 +1,4 @@ -# Generated by roxygen2 (4.1.0.9000): do not edit by hand +# Generated by roxygen2 (4.1.1): do not edit by hand export(define) export(packages) diff --git a/man/Ramd.Rd b/man/Ramd.Rd index fe31530..ea5352a 100644 --- a/man/Ramd.Rd +++ b/man/Ramd.Rd @@ -1,4 +1,4 @@ -% Generated by roxygen2 (4.1.0.9000): do not edit by hand +% Generated by roxygen2 (4.1.1): do not edit by hand % Please edit documentation in R/Ramd-package.r \docType{package} \name{Ramd} diff --git a/man/define.Rd b/man/define.Rd index 949ae8b..9259b2f 100644 --- a/man/define.Rd +++ b/man/define.Rd @@ -1,4 +1,4 @@ -% Generated by roxygen2 (4.1.0.9000): do not edit by hand +% Generated by roxygen2 (4.1.1): do not edit by hand % Please edit documentation in R/define.r \name{define} \alias{define} diff --git a/man/load_dependency.Rd b/man/load_dependency.Rd index 21a8bd2..4191833 100644 --- a/man/load_dependency.Rd +++ b/man/load_dependency.Rd @@ -1,4 +1,4 @@ -% Generated by roxygen2 (4.1.0.9000): do not edit by hand +% Generated by roxygen2 (4.1.1): do not edit by hand % Please edit documentation in R/load_dependency.r \name{load_dependency} \alias{load_dependency} diff --git a/man/load_package.Rd b/man/load_package.Rd index bf032ed..d1e9090 100644 --- a/man/load_package.Rd +++ b/man/load_package.Rd @@ -1,20 +1,21 @@ -% Generated by roxygen2 (4.1.0.9000): do not edit by hand +% Generated by roxygen2 (4.1.1): do not edit by hand % Please edit documentation in R/load_package.r \name{load_package} \alias{load_package} \title{Load a package and install it if necessary} \usage{ -load_package(name) +load_package(name, verbose = FALSE) } \arguments{ -\item{name}{Name of package} +\item{name}{character. Name of package.} + +\item{verbose}{logical. Whether or not to announce each installation.} } \description{ Load a package and install it if necessary } \examples{ \dontrun{ -load_package('glmnet') } } diff --git a/man/packages.Rd b/man/packages.Rd index 33f371c..5ffe702 100644 --- a/man/packages.Rd +++ b/man/packages.Rd @@ -1,22 +1,31 @@ -% Generated by roxygen2 (4.1.0.9000): do not edit by hand +% Generated by roxygen2 (4.1.1): do not edit by hand % Please edit documentation in R/packages.r \name{packages} \alias{packages} \title{Load a bunch of packages} \usage{ -packages(...) +packages(..., verbose = FALSE) } \arguments{ \item{...}{List of packages} + +\item{verbose}{logical. Whether or not to announce installations and +package startup messages.} +} +\value{ +TRUE } \description{ Load a bunch of packages } \examples{ \dontrun{ -packages('glmnet', 'caret) -packages('glmnet caret') -packages(list('glmnet', 'caret'), c('e1071', 'parallel multicore'), 'stringr') +packages("glmnet", "caret") +packages("glmnet caret") # Can just separate with a space +packages(list("glmnet", "caret"), c("e1071", "parallel multicore"), "stringr") # can nest space and vectorization +packages("robertzk/Ramd") # Can install from GitHub +packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") +packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub } } diff --git a/man/pp.Rd b/man/pp.Rd index 7b05b73..14b3089 100644 --- a/man/pp.Rd +++ b/man/pp.Rd @@ -1,4 +1,4 @@ -% Generated by roxygen2 (4.1.0.9000): do not edit by hand +% Generated by roxygen2 (4.1.1): do not edit by hand % Please edit documentation in R/pp.r \name{pp} \alias{pp} From 476d411736e6e64131de18254ed85d39fead15b2 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 17:43:31 -0600 Subject: [PATCH 04/21] check if a package is installed by referencing utils::installed.packages --- R/load_package.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/load_package.r b/R/load_package.r index ce37a54..56ad130 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -34,7 +34,7 @@ package_is_installed <- function(name) { if (is_github_package(name)) { name <- strsplit("robertzk/Ramd", "/")[[1]][[2]] } - require(name, character.only = TRUE) + name %in% utils::installed.packages()[,1] } is_github_package <- function(name) { From 62880e952e7c6c85f4ceeaf5a4628f67a44fc42a Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 17:44:03 -0600 Subject: [PATCH 05/21] don't split on @ to allow for refs --- R/packages.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/packages.r b/R/packages.r index d7fcdde..bcfad3b 100644 --- a/R/packages.r +++ b/R/packages.r @@ -15,7 +15,7 @@ #' } #' @return TRUE packages <- function(..., verbose = FALSE) { - split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/]+')[[1]] + split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/@]+')[[1]] pkgs <- unlist(lapply(unlist(list(...)), split_packages)) if (isTRUE(verbose)) { sapply(pkgs, load_package, verbose = TRUE) } else { suppressPackageStartupMessages(sapply(pkgs, load_package)) } From b3b264c3a9ee5e823bdd111bded54cc596bd560d Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 18:54:11 -0600 Subject: [PATCH 06/21] Handle version mismatching; add an impressive array of test coverage --- R/load_package.r | 44 ++++++++- R/packages.r | 2 +- tests/testthat/test-load_pacakge.r | 142 +++++++++++++++++++++++++++++ tests/testthat/test-packages.r | 68 ++++++++++++++ 4 files changed, 252 insertions(+), 4 deletions(-) create mode 100644 tests/testthat/test-load_pacakge.r create mode 100644 tests/testthat/test-packages.r diff --git a/R/load_package.r b/R/load_package.r index 56ad130..dcea3c4 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -12,8 +12,14 @@ load_package <- function(name, verbose = FALSE) { if (isTRUE(verbose)) { message(name, " already installed.") } return(TRUE) } + if (is_version_mismatch(name)) { + if (isTRUE(verbose)) { + message("Removing prior insallation of ", name_from_github_name(name)) + } + remove.packages(name) + } if (is_github_package(name)) { - remote <- "github" + remote <- "GitHub" if (isTRUE(verbose)) { announce(name, remote) } devtools::install_github(name) } else { @@ -22,21 +28,53 @@ load_package <- function(name, verbose = FALSE) { install.packages(name) # install from CRAN } if (!package_is_installed(name)) { - stop(paste('Package', name, "not found on", remote, ".")) + stop(paste("Package", name, "not found on", remote, ".")) } } + announce <- function(name, remote) { message("Installing ", name, " from ", remote, ".") } + package_is_installed <- function(name) { if (is_github_package(name)) { - name <- strsplit("robertzk/Ramd", "/")[[1]][[2]] + name <- name_from_github_name(name) } name %in% utils::installed.packages()[,1] } + +name_from_github_name <- function(name) { + strsplit(strsplit(name, "/")[[1]][[2]], "@")[[1]][[1]] +} + + is_github_package <- function(name) { + # Checks for github repos, e.g., robertzk/Ramd grepl("/", name, fixed = TRUE) } + + +is_version_mismatch <- function(name) { + is_versionable <- function(name) { + grepl("@", name, fixed = TRUE) && + grepl(".", name, fixed = TRUE) + } + get_version_from_ref <- function(name) { + # extract 0.3 from robertzk/Ramd@v0.3 + name <- strsplit(name, "@")[[1]][[2]] + if (grepl("v", name, fixed = TRUE)) { + name <- strsplit(name, "v")[[1]][[2]] + } + name + } + + is_version_mismatch <- function(name) { + packageVersion(name) != package_version(get_version_from_ref(name)) + } + + # Checks for specified refs or package names, e.g. robertzk/Ramd@v0.3 + is_github_package(name) && is_versionable(name) && is_version_mismatch(name) +} diff --git a/R/packages.r b/R/packages.r index bcfad3b..b15211a 100644 --- a/R/packages.r +++ b/R/packages.r @@ -16,7 +16,7 @@ #' @return TRUE packages <- function(..., verbose = FALSE) { split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/@]+')[[1]] - pkgs <- unlist(lapply(unlist(list(...)), split_packages)) + pkgs <- unique(unlist(lapply(unlist(list(...)), split_packages))) if (isTRUE(verbose)) { sapply(pkgs, load_package, verbose = TRUE) } else { suppressPackageStartupMessages(sapply(pkgs, load_package)) } TRUE diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r new file mode 100644 index 0000000..78cf2d1 --- /dev/null +++ b/tests/testthat/test-load_pacakge.r @@ -0,0 +1,142 @@ +context("load_package") +library(testthatsomemore) + +describe("package already installed", { + describe("within load_package", { + with_mock( + `package_is_installed` = function(...) TRUE, + `devtools::install_github` = function(...) { stop("No installing!") }, + `install.packages` = function(...) { stop("No installing!") }, { + test_that("it will not install if already installed", { + expect_true(load_package("glmnet")) + }) + test_that("it messages if it is already installed and verbose is TRUE", { + expect_message(load_package("glmnet", verbose = TRUE), "already installed") + }) + test_that("it does not message if it is already installed and verbose is FALSE", { + expect_silent(load_package("glmnet", verbose = FALSE)) + }) + }) + }) + describe("package_is_installed helper", { + with_mock(`utils::installed.packages` = function() { data.frame(name = "glmnet") }, { + test_that("it works", { + expect_true(package_is_installed("glmnet")) + expect_false(package_is_installed("Ramd")) + }) + }) + }) +}) + +describe("version mismatching", { + describe("within load_package", { + test_that("it removes the package if there is a version mismatch", { + with_mock( + `is_version_mismatch` = function(name) { TRUE }, + `devtools::install_github` = function(...) { "No installing!" }, + `install.packages` = function(...) { "No installing!" }, + `remove.packages` = function(name) { called <<- TRUE }, { + called <<- FALSE + expect_false(called) + expect_error(load_package("glmnet")) # because glmnet is not actually installed + expect_true(called) + }) + }) + }) + test_that("it does not remove the package if there is not a version mismatch", { + with_mock( + `is_version_mismatch` = function(name) { FALSE }, + `devtools::install_github` = function(...) { "No installing!" }, + `install.packages` = function(...) { "No installing!" }, + `remove.packages` = function(name) { called <<- TRUE }, { + called <<- FALSE + expect_false(called) + expect_error(load_package("glmnet")) # because glmnet is not actually installed + expect_false(called) + }) + }) + }) + + describe("is_version_mismatch helper function", { + with_mock( + `packageVersion` = function(name) { package_version("1.1") }, { + expect_true(is_version_mismatch("robertzk/Ramd@v1.0")) + expect_true(is_version_mismatch("robertzk/Ramd@v1.2")) + expect_true(is_version_mismatch("robertzk/Ramd@v1.1.3")) + expect_true(is_version_mismatch("robertzk/Ramd@1.0")) + expect_false(is_version_mismatch("robertzk/Ramd@v1.1")) + expect_false(is_version_mismatch("robertzk/Ramd@1.1")) + expect_false(is_version_mismatch("robertzk/Ramd")) + expect_false(is_version_mismatch("Ramd")) + } + ) + }) + +describe("it can install from CRAN", { + with_mock( + `devtools::install_github` = function(...) { stop("Wrong installer!") }, + `stop` = function(...) { TRUE }, #Avoid crashing since we aren't really installing + `install.packages` = function(...) { "correct installer!" }, { + test_that("it installs", { + expect_true(load_package("glmnet")) + }) + test_that("it messages if verbose is TRUE", { + expect_message(load_package("glmnet", verbose = TRUE), "Installing") + }) + test_that("it messages the remote correctly", { + expect_message(load_package("glmnet", verbose = TRUE), "CRAN") + }) + test_that("it does not message if verbose is FALSE", { + expect_silent(load_package("glmnet", verbose = FALSE)) + }) + }) + with_mock( + `devtools::install_github` = function(...) { stop("Wrong installer!") }, + `install.packages` = function(...) { "correct installer!" }, + `package_is_installed` = function(...) { FALSE }, { + test_that("if package isn't on CRAN, that's an error", { + expect_error(load_package("bozo"), "not found") + }) + }) + }) + +describe("it can install from GitHub", { + describe("within load_package", { + with_mock( + `devtools::install_github` = function(...) { "Correct installer!" }, + `package_is_installed` = function(...) { FALSE }, + `stop` = function(...) { TRUE }, #Avoid crashing since we aren't really installing + `install.packages` = function(...) { stop("Wrong installer!") }, { + test_that("it installs", { + expect_true(load_package("robertzk/Ramd")) + }) + test_that("it messages the remote correctly", { + expect_message(load_package("robertzk/Ramd", verbose = TRUE), "GitHub") + }) + }) + with_mock( + `devtools::install_github` = function(...) { "Correct installer" }, + `install.packages` = function(...) { stop("Wrong installer!") }, + `package_is_installed` = function(...) { FALSE }, { + test_that("if package isn't on CRAN, that's an error", { + expect_error(load_package("bozo/bozo"), "not found") + }) + }) + }) + describe("is_github_package helper function", { + expect_true(is_github_package("hadley/dplyr")) + expect_true(is_github_package("robertzk/Ramd")) + expect_true(is_github_package("wch/R6@v1.1")) + expect_true(is_github_package("peterhurford/batchman@1.1")) + expect_true(is_github_package("hadley/ggplot2@ref123abc")) + expect_false(is_github_package("glmnet")) + expect_false(is_github_package("dplyr")) + }) + test_that("name_from_github_name helper function", { + expect_equal("dplyr", name_from_github_name("hadley/dplyr")) + expect_equal("Ramd", name_from_github_name("robertzk/Ramd")) + expect_equal("R6", name_from_github_name("wch/R6@v1.1")) + expect_equal("batchman", name_from_github_name("peterhurford/batchman@1.1")) + expect_equal("ggplot2", name_from_github_name("hadley/ggplot2@ref123abc")) + }) +}) diff --git a/tests/testthat/test-packages.r b/tests/testthat/test-packages.r new file mode 100644 index 0000000..728dde5 --- /dev/null +++ b/tests/testthat/test-packages.r @@ -0,0 +1,68 @@ +context("packages") +library(testthatsomemore) + +with_mock( + `load_package` = function(package) { install_count <<- install_count + 1; package }, { + test_that("it can install from CRAN", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("glmnet")) + expect_equal(1, install_count) + }) + + test_that("it can install from GitHub", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("robertzk/Ramd")) + expect_equal(1, install_count) + }) + + test_that("it can install twice from CRAN", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("glmnet", "dplyr")) + expect_equal(2, install_count) + }) + + test_that("it can install twice from GitHub", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("robertzk/Ramd", "hadley/dplyr")) + expect_equal(2, install_count) + }) + + test_that("it can install twice from a mix of both remotes", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("robertzk/Ramd", "dplyr")) + expect_equal(2, install_count) + }) + + test_that("it can install three times", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("robertzk/Ramd", "dplyr", "glmnet")) + expect_equal(3, install_count) + }) + + test_that("it can install space separated", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("robertzk/Ramd dplyr glmnet")) + expect_equal(3, install_count) + }) + + test_that("it can install on a package version", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("robertzk/Ramd@v0.3 dplyr@1.0 glmnet@abc123")) + expect_equal(3, install_count) + }) + + test_that("it installs uniquely", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages("glmnet glmnet dplyr dplyr")) + expect_equal(2, install_count) + }) + }) From 1c5392f698435208f57cf4d3197e29462b4a90dd Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 18:54:40 -0600 Subject: [PATCH 07/21] announce test coverage --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 8a731e7..68e7476 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ * `packages` will now install from GitHub when the package name contains a '/'. * `packages` will now take `verbose = TRUE` to announce the installations. + * Add test coverage for `packages` and `load_package`. # Version 0.2.1 From 6aba417be543f2d95fd5387d843ec1948faa770e Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:06:54 -0600 Subject: [PATCH 08/21] Can install from a github subdir --- R/load_package.r | 13 ++++++++++++- R/packages.r | 14 +++++++++++--- tests/testthat/test-load_pacakge.r | 22 ++++++++++++++++------ tests/testthat/test-packages.r | 16 +++++++++++++++- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/R/load_package.r b/R/load_package.r index dcea3c4..ce8c6a5 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -8,6 +8,12 @@ #" load_package("glmnet") #' } load_package <- function(name, verbose = FALSE) { + if (is.list(name)) { + metadata <- name[-1] # For tracking things like subdir + name <- name[[1]] + } else { + metadata <- NULL + } if (package_is_installed(name)) { if (isTRUE(verbose)) { message(name, " already installed.") } return(TRUE) @@ -21,7 +27,11 @@ load_package <- function(name, verbose = FALSE) { if (is_github_package(name)) { remote <- "GitHub" if (isTRUE(verbose)) { announce(name, remote) } - devtools::install_github(name) + if (!is.null(metadata)) { + do.call(devtools::install_github, c(name, metadata)) + } else { + devtools::install_github(name) + } } else { remote <- "CRAN" if (isTRUE(verbose)) { announce(name, remote) } @@ -30,6 +40,7 @@ load_package <- function(name, verbose = FALSE) { if (!package_is_installed(name)) { stop(paste("Package", name, "not found on", remote, ".")) } + TRUE } diff --git a/R/packages.r b/R/packages.r index b15211a..778986e 100644 --- a/R/packages.r +++ b/R/packages.r @@ -10,14 +10,22 @@ #' packages("glmnet caret") # Can just separate with a space #' packages(list("glmnet", "caret"), c("e1071", "parallel multicore"), "stringr") # can nest space and vectorization #' packages("robertzk/Ramd") # Can install from GitHub +#' packages("robertzk/Ramd@v1.1") # Can install a particular version +#' packages("robertzk/Ramd@03bbd4120ba39cfb772485ca9f47aa5c91b2dc48") # or a ref +#' packages(list("FeiYeYe/xgboost", subdir = "R-package")) # or a subdir #' packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") #' packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub #' } #' @return TRUE packages <- function(..., verbose = FALSE) { split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/@]+')[[1]] - pkgs <- unique(unlist(lapply(unlist(list(...)), split_packages))) - if (isTRUE(verbose)) { sapply(pkgs, load_package, verbose = TRUE) } - else { suppressPackageStartupMessages(sapply(pkgs, load_package)) } + if ("list" %in% vapply(list(...), class, character(1))) { + pkgs <- list(...) + } else { + pkgs <- unlist(lapply(unlist(list(...)), split_packages)) + } + pkgs <- unique(pkgs) + if (isTRUE(verbose)) { vapply(pkgs, load_package, verbose = TRUE, logical(1)) } + else { suppressPackageStartupMessages(vapply(pkgs, load_package, logical(1))) } TRUE } diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r index 78cf2d1..c15b035 100644 --- a/tests/testthat/test-load_pacakge.r +++ b/tests/testthat/test-load_pacakge.r @@ -113,12 +113,22 @@ describe("it can install from GitHub", { test_that("it messages the remote correctly", { expect_message(load_package("robertzk/Ramd", verbose = TRUE), "GitHub") }) - }) - with_mock( - `devtools::install_github` = function(...) { "Correct installer" }, - `install.packages` = function(...) { stop("Wrong installer!") }, - `package_is_installed` = function(...) { FALSE }, { - test_that("if package isn't on CRAN, that's an error", { + }) + with_mock( + `devtools::install_github` = function(...) { captured_args <<- list(...) }, + `package_is_installed` = function(...) { FALSE }, + `install.packages` = function(...) { stop("Wrong installer!") }, { + test_that("it installs from a subdir", { + expect_error(load_package(list("FeiYeYe/xgboost", subdir = "R-package"))) + expect_equal(captured_args[[1]], "FeiYeYe/xgboost") + expect_equal(captured_args$subdir, "R-package") + }) + }) + with_mock( + `devtools::install_github` = function(...) { "Correct installer" }, + `install.packages` = function(...) { stop("Wrong installer!") }, + `package_is_installed` = function(...) { FALSE }, { + test_that("if package isn't on CRAN, that's an error", { expect_error(load_package("bozo/bozo"), "not found") }) }) diff --git a/tests/testthat/test-packages.r b/tests/testthat/test-packages.r index 728dde5..bf8ad31 100644 --- a/tests/testthat/test-packages.r +++ b/tests/testthat/test-packages.r @@ -2,7 +2,7 @@ context("packages") library(testthatsomemore) with_mock( - `load_package` = function(package) { install_count <<- install_count + 1; package }, { + `load_package` = function(package) { install_count <<- install_count + 1; TRUE }, { test_that("it can install from CRAN", { install_count <<- 0 expect_equal(0, install_count) @@ -59,6 +59,20 @@ with_mock( expect_equal(3, install_count) }) + test_that("it can install a subdir", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages(list("FeiYeYe/xgboost", subdir = "R-package"))) + expect_equal(1, install_count) + }) + + test_that("it can install a subdir and something else", { + install_count <<- 0 + expect_equal(0, install_count) + expect_true(packages(list("FeiYeYe/xgboost", subdir = "R-package"), "glmnet")) + expect_equal(2, install_count) + }) + test_that("it installs uniquely", { install_count <<- 0 expect_equal(0, install_count) From 785b5582b23b9a872b0e44a90cd39886ac6e3758 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:08:32 -0600 Subject: [PATCH 09/21] Fix travis yml to use development testthat for with_mock --- .travis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 5c6178a..f939cd2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,8 +5,9 @@ before_install: - ./travis-tool.sh bootstrap install: - ./travis-tool.sh install_deps -- ./travis-tool.sh install_r testthat testthatsomemore -- "./travis-tool.sh github_package jimhester/covr" +- Rscript -e 'if (!require("covr")) devtools::install_github("kirillseva/covr")' +- Rscript -e 'if (!require("testthat")) devtools::install_github("hadley/testthat")' +- Rscript -e 'if (!require("testthatsomemore")) devtools::install_github("robertzk/testthatsomemore")' script: ./travis-tool.sh run_tests after_failure: - ./travis-tool.sh dump_logs From aa00ea8702fcacf7bf7d4ced03cd2fc3a99eb6d6 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:25:44 -0600 Subject: [PATCH 10/21] Ensure devtools installed; fix docs; fix tests --- R/load_package.r | 9 +-------- R/packages.r | 7 +------ R/utils.r | 11 +++++++++++ man/packages.Rd | 4 ---- tests/testthat/test-load_pacakge.r | 5 ++++- 5 files changed, 17 insertions(+), 19 deletions(-) create mode 100644 R/utils.r diff --git a/R/load_package.r b/R/load_package.r index ce8c6a5..98ef2d9 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -25,6 +25,7 @@ load_package <- function(name, verbose = FALSE) { remove.packages(name) } if (is_github_package(name)) { + ensure_devtools_installed() remote <- "GitHub" if (isTRUE(verbose)) { announce(name, remote) } if (!is.null(metadata)) { @@ -49,14 +50,6 @@ announce <- function(name, remote) { } -package_is_installed <- function(name) { - if (is_github_package(name)) { - name <- name_from_github_name(name) - } - name %in% utils::installed.packages()[,1] -} - - name_from_github_name <- function(name) { strsplit(strsplit(name, "/")[[1]][[2]], "@")[[1]][[1]] } diff --git a/R/packages.r b/R/packages.r index 778986e..503982f 100644 --- a/R/packages.r +++ b/R/packages.r @@ -3,20 +3,15 @@ #' @param ... List of packages #' @param verbose logical. Whether or not to announce installations and #' package startup messages. -#' @export #' @examples #' \dontrun{ #' packages("glmnet", "caret") #' packages("glmnet caret") # Can just separate with a space -#' packages(list("glmnet", "caret"), c("e1071", "parallel multicore"), "stringr") # can nest space and vectorization #' packages("robertzk/Ramd") # Can install from GitHub -#' packages("robertzk/Ramd@v1.1") # Can install a particular version -#' packages("robertzk/Ramd@03bbd4120ba39cfb772485ca9f47aa5c91b2dc48") # or a ref -#' packages(list("FeiYeYe/xgboost", subdir = "R-package")) # or a subdir #' packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") #' packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub #' } -#' @return TRUE +#' @export packages <- function(..., verbose = FALSE) { split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/@]+')[[1]] if ("list" %in% vapply(list(...), class, character(1))) { diff --git a/R/utils.r b/R/utils.r new file mode 100644 index 0000000..64e8ea0 --- /dev/null +++ b/R/utils.r @@ -0,0 +1,11 @@ +ensure_devtools_installed <- function() { + package_is_installed("devtools") + stop("Devtools must be installed first. Run `install.packages('devtools')`.") +} + +package_is_installed <- function(name) { + if (is_github_package(name)) { + name <- name_from_github_name(name) + } + name %in% utils::installed.packages()[,1] +} diff --git a/man/packages.Rd b/man/packages.Rd index 5ffe702..3f02a98 100644 --- a/man/packages.Rd +++ b/man/packages.Rd @@ -12,9 +12,6 @@ packages(..., verbose = FALSE) \item{verbose}{logical. Whether or not to announce installations and package startup messages.} } -\value{ -TRUE -} \description{ Load a bunch of packages } @@ -22,7 +19,6 @@ Load a bunch of packages \dontrun{ packages("glmnet", "caret") packages("glmnet caret") # Can just separate with a space -packages(list("glmnet", "caret"), c("e1071", "parallel multicore"), "stringr") # can nest space and vectorization packages("robertzk/Ramd") # Can install from GitHub packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r index c15b035..ab4b12b 100644 --- a/tests/testthat/test-load_pacakge.r +++ b/tests/testthat/test-load_pacakge.r @@ -93,6 +93,7 @@ describe("it can install from CRAN", { with_mock( `devtools::install_github` = function(...) { stop("Wrong installer!") }, `install.packages` = function(...) { "correct installer!" }, + `ensure_devtools_installed` = function(...) { TRUE }, `package_is_installed` = function(...) { FALSE }, { test_that("if package isn't on CRAN, that's an error", { expect_error(load_package("bozo"), "not found") @@ -117,6 +118,7 @@ describe("it can install from GitHub", { with_mock( `devtools::install_github` = function(...) { captured_args <<- list(...) }, `package_is_installed` = function(...) { FALSE }, + `ensure_devtools_installed` = function(...) { TRUE }, `install.packages` = function(...) { stop("Wrong installer!") }, { test_that("it installs from a subdir", { expect_error(load_package(list("FeiYeYe/xgboost", subdir = "R-package"))) @@ -126,9 +128,10 @@ describe("it can install from GitHub", { }) with_mock( `devtools::install_github` = function(...) { "Correct installer" }, + `ensure_devtools_installed` = function(...) { TRUE }, `install.packages` = function(...) { stop("Wrong installer!") }, `package_is_installed` = function(...) { FALSE }, { - test_that("if package isn't on CRAN, that's an error", { + test_that("if package isn't on GitHub, that's an error", { expect_error(load_package("bozo/bozo"), "not found") }) }) From fadd21511a9f7c3b5ac04f0a47056a4a865c761b Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:33:20 -0600 Subject: [PATCH 11/21] suggest devtools --- DESCRIPTION | 1 + 1 file changed, 1 insertion(+) diff --git a/DESCRIPTION b/DESCRIPTION index dde5273..d781933 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -11,6 +11,7 @@ Depends: R (>= 3.0.0) Suggests: testthat, + devtools, testthatsomemore License: GPL (>= 2) LazyData: true From ab306f345e8b964015e6da97f3db673e086eeb99 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:37:21 -0600 Subject: [PATCH 12/21] Mention license in DESCRIPTION to make R CMD CHECK happy --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index d781933..fb9767f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -13,5 +13,5 @@ Suggests: testthat, devtools, testthatsomemore -License: GPL (>= 2) +License: MIT + file LICENSE LazyData: true From c5ded6233ee599dd4fb59e89f44ce7ffcdbf65ca Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:43:21 -0600 Subject: [PATCH 13/21] Use enough prefixing to make R CMD CHECK happy --- R/load_package.r | 4 +-- tests/testthat/test-load_pacakge.r | 40 +++++++++++++++--------------- tests/testthat/test-packages.r | 2 +- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/R/load_package.r b/R/load_package.r index 98ef2d9..bb02d81 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -22,7 +22,7 @@ load_package <- function(name, verbose = FALSE) { if (isTRUE(verbose)) { message("Removing prior insallation of ", name_from_github_name(name)) } - remove.packages(name) + utils::remove.packages(name) } if (is_github_package(name)) { ensure_devtools_installed() @@ -36,7 +36,7 @@ load_package <- function(name, verbose = FALSE) { } else { remote <- "CRAN" if (isTRUE(verbose)) { announce(name, remote) } - install.packages(name) # install from CRAN + utils::install.packages(name) # install from CRAN } if (!package_is_installed(name)) { stop(paste("Package", name, "not found on", remote, ".")) diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r index ab4b12b..4423c40 100644 --- a/tests/testthat/test-load_pacakge.r +++ b/tests/testthat/test-load_pacakge.r @@ -4,9 +4,9 @@ library(testthatsomemore) describe("package already installed", { describe("within load_package", { with_mock( - `package_is_installed` = function(...) TRUE, + `Ramd:::package_is_installed` = function(...) TRUE, `devtools::install_github` = function(...) { stop("No installing!") }, - `install.packages` = function(...) { stop("No installing!") }, { + `utils::install.packages` = function(...) { stop("No installing!") }, { test_that("it will not install if already installed", { expect_true(load_package("glmnet")) }) @@ -32,10 +32,10 @@ describe("version mismatching", { describe("within load_package", { test_that("it removes the package if there is a version mismatch", { with_mock( - `is_version_mismatch` = function(name) { TRUE }, + `Ramd:::is_version_mismatch` = function(name) { TRUE }, `devtools::install_github` = function(...) { "No installing!" }, - `install.packages` = function(...) { "No installing!" }, - `remove.packages` = function(name) { called <<- TRUE }, { + `utils::install.packages` = function(...) { "No installing!" }, + `utils::remove.packages` = function(name) { called <<- TRUE }, { called <<- FALSE expect_false(called) expect_error(load_package("glmnet")) # because glmnet is not actually installed @@ -45,10 +45,10 @@ describe("version mismatching", { }) test_that("it does not remove the package if there is not a version mismatch", { with_mock( - `is_version_mismatch` = function(name) { FALSE }, + `Ramd:::is_version_mismatch` = function(name) { FALSE }, `devtools::install_github` = function(...) { "No installing!" }, - `install.packages` = function(...) { "No installing!" }, - `remove.packages` = function(name) { called <<- TRUE }, { + `utils::install.packages` = function(...) { "No installing!" }, + `utils::remove.packages` = function(name) { called <<- TRUE }, { called <<- FALSE expect_false(called) expect_error(load_package("glmnet")) # because glmnet is not actually installed @@ -76,7 +76,7 @@ describe("it can install from CRAN", { with_mock( `devtools::install_github` = function(...) { stop("Wrong installer!") }, `stop` = function(...) { TRUE }, #Avoid crashing since we aren't really installing - `install.packages` = function(...) { "correct installer!" }, { + `utils::install.packages` = function(...) { "correct installer!" }, { test_that("it installs", { expect_true(load_package("glmnet")) }) @@ -92,9 +92,9 @@ describe("it can install from CRAN", { }) with_mock( `devtools::install_github` = function(...) { stop("Wrong installer!") }, - `install.packages` = function(...) { "correct installer!" }, - `ensure_devtools_installed` = function(...) { TRUE }, - `package_is_installed` = function(...) { FALSE }, { + `utils::install.packages` = function(...) { "correct installer!" }, + `Ramd:::ensure_devtools_installed` = function(...) { TRUE }, + `Ramd:::package_is_installed` = function(...) { FALSE }, { test_that("if package isn't on CRAN, that's an error", { expect_error(load_package("bozo"), "not found") }) @@ -105,9 +105,9 @@ describe("it can install from GitHub", { describe("within load_package", { with_mock( `devtools::install_github` = function(...) { "Correct installer!" }, - `package_is_installed` = function(...) { FALSE }, + `Ramd:::package_is_installed` = function(...) { FALSE }, `stop` = function(...) { TRUE }, #Avoid crashing since we aren't really installing - `install.packages` = function(...) { stop("Wrong installer!") }, { + `utils::install.packages` = function(...) { stop("Wrong installer!") }, { test_that("it installs", { expect_true(load_package("robertzk/Ramd")) }) @@ -117,9 +117,9 @@ describe("it can install from GitHub", { }) with_mock( `devtools::install_github` = function(...) { captured_args <<- list(...) }, - `package_is_installed` = function(...) { FALSE }, - `ensure_devtools_installed` = function(...) { TRUE }, - `install.packages` = function(...) { stop("Wrong installer!") }, { + `Ramd:::package_is_installed` = function(...) { FALSE }, + `Ramd:::ensure_devtools_installed` = function(...) { TRUE }, + `utils::install.packages` = function(...) { stop("Wrong installer!") }, { test_that("it installs from a subdir", { expect_error(load_package(list("FeiYeYe/xgboost", subdir = "R-package"))) expect_equal(captured_args[[1]], "FeiYeYe/xgboost") @@ -128,9 +128,9 @@ describe("it can install from GitHub", { }) with_mock( `devtools::install_github` = function(...) { "Correct installer" }, - `ensure_devtools_installed` = function(...) { TRUE }, - `install.packages` = function(...) { stop("Wrong installer!") }, - `package_is_installed` = function(...) { FALSE }, { + `Ramd:::ensure_devtools_installed` = function(...) { TRUE }, + `utils::install.packages` = function(...) { stop("Wrong installer!") }, + `Ramd:::package_is_installed` = function(...) { FALSE }, { test_that("if package isn't on GitHub, that's an error", { expect_error(load_package("bozo/bozo"), "not found") }) diff --git a/tests/testthat/test-packages.r b/tests/testthat/test-packages.r index bf8ad31..cbe4a18 100644 --- a/tests/testthat/test-packages.r +++ b/tests/testthat/test-packages.r @@ -2,7 +2,7 @@ context("packages") library(testthatsomemore) with_mock( - `load_package` = function(package) { install_count <<- install_count + 1; TRUE }, { + `Ramd:::load_package` = function(package) { install_count <<- install_count + 1; TRUE }, { test_that("it can install from CRAN", { install_count <<- 0 expect_equal(0, install_count) From 93b5085c675b208f10515e207b26fa033941c30d Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Tue, 19 Jan 2016 19:48:57 -0600 Subject: [PATCH 14/21] prefix packageVersion --- R/load_package.r | 2 +- tests/testthat/test-load_pacakge.r | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/load_package.r b/R/load_package.r index bb02d81..c598e14 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -76,7 +76,7 @@ is_version_mismatch <- function(name) { } is_version_mismatch <- function(name) { - packageVersion(name) != package_version(get_version_from_ref(name)) + utils::packageVersion(name) != package_version(get_version_from_ref(name)) } # Checks for specified refs or package names, e.g. robertzk/Ramd@v0.3 diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r index 4423c40..7e71fe9 100644 --- a/tests/testthat/test-load_pacakge.r +++ b/tests/testthat/test-load_pacakge.r @@ -59,7 +59,7 @@ describe("version mismatching", { describe("is_version_mismatch helper function", { with_mock( - `packageVersion` = function(name) { package_version("1.1") }, { + `utils::packageVersion` = function(name) { package_version("1.1") }, { expect_true(is_version_mismatch("robertzk/Ramd@v1.0")) expect_true(is_version_mismatch("robertzk/Ramd@v1.2")) expect_true(is_version_mismatch("robertzk/Ramd@v1.1.3")) From 0d0a5b9d63e7f5e59ef7bca23e5c56ff4fe287b0 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 10:17:16 -0600 Subject: [PATCH 15/21] Fix docs --- R/load_package.r | 7 ++++++- R/packages.r | 13 +++++++++---- man/load_package.Rd | 6 ++++++ man/packages.Rd | 13 +++++++++---- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/R/load_package.r b/R/load_package.r index c598e14..c017b6e 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -5,7 +5,12 @@ #' @param verbose logical. Whether or not to announce each installation. #' @examples #' \dontrun{ -#" load_package("glmnet") +#' load_package("glmnet") +#' load_package("robertzk/Ramd") +#' load_package("robertzk/Ramd@@v0.3") # Can load from versions +#' load_package("robertzk/Ramd@@0.3") +#' load_package("robertzk/Ramd@@fbe1aa0e36df289b27881d077635352e6debdbc1") # Can load from refs +#' load_package(list("FeiYeYe/xgboost", subdir = "R-package")) # Can load from subdirectories #' } load_package <- function(name, verbose = FALSE) { if (is.list(name)) { diff --git a/R/packages.r b/R/packages.r index 503982f..6440fd4 100644 --- a/R/packages.r +++ b/R/packages.r @@ -5,11 +5,16 @@ #' package startup messages. #' @examples #' \dontrun{ -#' packages("glmnet", "caret") -#' packages("glmnet caret") # Can just separate with a space -#' packages("robertzk/Ramd") # Can install from GitHub -#' packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") +#' packages("glmnet") # Can install from CRAN +#' packages("robertzk/Ramd") # Can install from GitHub +#' packages("glmnet", "caret") # can load multiple packages in one call +#' packages("glmnet caret") # Can just separate with a space +#' packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") # Can load multiple #' packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub +#' packages("robertzk/Ramd@@v0.3") # Can load from versions +#' packages("robertzk/Ramd@@0.3") +#' packages("robertzk/Ramd@@fbe1aa0e36df289b27881d077635352e6debdbc1") # Can load from refs +#' packages(list("FeiYeYe/xgboost", subdir = "R-package")) # Can load from subdirectories #' } #' @export packages <- function(..., verbose = FALSE) { diff --git a/man/load_package.Rd b/man/load_package.Rd index d1e9090..bb13e0f 100644 --- a/man/load_package.Rd +++ b/man/load_package.Rd @@ -16,6 +16,12 @@ Load a package and install it if necessary } \examples{ \dontrun{ +load_package("glmnet") +load_package("robertzk/Ramd") +load_package("robertzk/Ramd@v0.3") # Can load from versions +load_package("robertzk/Ramd@0.3") +load_package("robertzk/Ramd@fbe1aa0e36df289b27881d077635352e6debdbc1") # Can load from refs +load_package(list("FeiYeYe/xgboost", subdir = "R-package")) # Can load from subdirectories } } diff --git a/man/packages.Rd b/man/packages.Rd index 3f02a98..2b57916 100644 --- a/man/packages.Rd +++ b/man/packages.Rd @@ -17,11 +17,16 @@ Load a bunch of packages } \examples{ \dontrun{ -packages("glmnet", "caret") -packages("glmnet caret") # Can just separate with a space -packages("robertzk/Ramd") # Can install from GitHub -packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") +packages("glmnet") # Can install from CRAN +packages("robertzk/Ramd") # Can install from GitHub +packages("glmnet", "caret") # can load multiple packages in one call +packages("glmnet caret") # Can just separate with a space +packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") # Can load multiple packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub +packages("robertzk/Ramd@v0.3") # Can load from versions +packages("robertzk/Ramd@0.3") +packages("robertzk/Ramd@fbe1aa0e36df289b27881d077635352e6debdbc1") # Can load from refs +packages(list("FeiYeYe/xgboost", subdir = "R-package")) # Can load from subdirectories } } From 6dec4663cebe66d32431a2d6ede01b3d86ec901c Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 13:02:55 -0600 Subject: [PATCH 16/21] Clean up metadata handling --- R/load_package.r | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/R/load_package.r b/R/load_package.r index c017b6e..33c8ea3 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -13,12 +13,9 @@ #' load_package(list("FeiYeYe/xgboost", subdir = "R-package")) # Can load from subdirectories #' } load_package <- function(name, verbose = FALSE) { - if (is.list(name)) { - metadata <- name[-1] # For tracking things like subdir - name <- name[[1]] - } else { - metadata <- NULL - } + metadata <- name[-1] # For tracking things like subdir + name <- name[[1]] + if (package_is_installed(name)) { if (isTRUE(verbose)) { message(name, " already installed.") } return(TRUE) @@ -33,8 +30,8 @@ load_package <- function(name, verbose = FALSE) { ensure_devtools_installed() remote <- "GitHub" if (isTRUE(verbose)) { announce(name, remote) } - if (!is.null(metadata)) { - do.call(devtools::install_github, c(name, metadata)) + if (length(metadata) > 0) { + do.call(devtools::install_github, c(list(name), metadata)) } else { devtools::install_github(name) } From 2606710d9578681d35e9a5156ac97a2ae4cac394 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 13:03:02 -0600 Subject: [PATCH 17/21] typofix --- R/load_package.r | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/load_package.r b/R/load_package.r index 33c8ea3..eae9639 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -22,7 +22,7 @@ load_package <- function(name, verbose = FALSE) { } if (is_version_mismatch(name)) { if (isTRUE(verbose)) { - message("Removing prior insallation of ", name_from_github_name(name)) + message("Removing prior installation of ", name_from_github_name(name)) } utils::remove.packages(name) } From 0a9b72e20acece57c936b7f50ad7155401f676e8 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 13:07:10 -0600 Subject: [PATCH 18/21] Split up load_package method --- R/load_package.r | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/R/load_package.r b/R/load_package.r index eae9639..e850015 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -20,32 +20,46 @@ load_package <- function(name, verbose = FALSE) { if (isTRUE(verbose)) { message(name, " already installed.") } return(TRUE) } - if (is_version_mismatch(name)) { - if (isTRUE(verbose)) { - message("Removing prior installation of ", name_from_github_name(name)) - } - utils::remove.packages(name) - } + + handle_version_mismatches(name, verbose) + if (is_github_package(name)) { - ensure_devtools_installed() remote <- "GitHub" - if (isTRUE(verbose)) { announce(name, remote) } - if (length(metadata) > 0) { - do.call(devtools::install_github, c(list(name), metadata)) - } else { - devtools::install_github(name) - } + install_from_github(name, metadata, remote, verbose) } else { remote <- "CRAN" - if (isTRUE(verbose)) { announce(name, remote) } - utils::install.packages(name) # install from CRAN + install_from_cran(name, remote, verbose) } + if (!package_is_installed(name)) { stop(paste("Package", name, "not found on", remote, ".")) } TRUE } +handle_version_mismatches <- function(name, verbose) { + if (is_version_mismatch(name)) { + if (isTRUE(verbose)) { + message("Removing prior installation of ", name_from_github_name(name)) + } + utils::remove.packages(name) + } +} + +install_from_github <- function(name, metadata, remote, verbose) { + ensure_devtools_installed() + if (isTRUE(verbose)) { announce(name, remote) } + if (length(metadata) > 0) { + do.call(devtools::install_github, c(list(name), metadata)) + } else { + devtools::install_github(name) + } +} + +install_from_cran <- function(name, remote, verbose) { + if (isTRUE(verbose)) { announce(name, remote) } + utils::install.packages(name) # install from CRAN +} announce <- function(name, remote) { message("Installing ", name, " from ", remote, ".") From c6bb7899a0de9b375223f34aa58c099f2d96ce92 Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 13:09:26 -0600 Subject: [PATCH 19/21] Deprecate space seperation --- NEWS.md | 1 + R/packages.r | 9 +-------- tests/testthat/test-packages.r | 11 ++--------- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/NEWS.md b/NEWS.md index 68e7476..6436bd0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,7 @@ * `packages` will now install from GitHub when the package name contains a '/'. * `packages` will now take `verbose = TRUE` to announce the installations. * Add test coverage for `packages` and `load_package`. + * Deprecate space separation for `packages` (instead, must pass multiple arguments). # Version 0.2.1 diff --git a/R/packages.r b/R/packages.r index 6440fd4..c8542d9 100644 --- a/R/packages.r +++ b/R/packages.r @@ -8,7 +8,6 @@ #' packages("glmnet") # Can install from CRAN #' packages("robertzk/Ramd") # Can install from GitHub #' packages("glmnet", "caret") # can load multiple packages in one call -#' packages("glmnet caret") # Can just separate with a space #' packages("robertzk/Ramd", "hadley/dplyr", "peterhurford/batchman") # Can load multiple #' packages("robertzk/Ramd", "glmnet") # Can install from both CRAN and GitHub #' packages("robertzk/Ramd@@v0.3") # Can load from versions @@ -18,13 +17,7 @@ #' } #' @export packages <- function(..., verbose = FALSE) { - split_packages <- function(string) strsplit(string, '[^a-zA-Z.$0-9_/@]+')[[1]] - if ("list" %in% vapply(list(...), class, character(1))) { - pkgs <- list(...) - } else { - pkgs <- unlist(lapply(unlist(list(...)), split_packages)) - } - pkgs <- unique(pkgs) + pkgs <- unique(list(...)) if (isTRUE(verbose)) { vapply(pkgs, load_package, verbose = TRUE, logical(1)) } else { suppressPackageStartupMessages(vapply(pkgs, load_package, logical(1))) } TRUE diff --git a/tests/testthat/test-packages.r b/tests/testthat/test-packages.r index cbe4a18..5a5130a 100644 --- a/tests/testthat/test-packages.r +++ b/tests/testthat/test-packages.r @@ -45,17 +45,10 @@ with_mock( expect_equal(3, install_count) }) - test_that("it can install space separated", { - install_count <<- 0 - expect_equal(0, install_count) - expect_true(packages("robertzk/Ramd dplyr glmnet")) - expect_equal(3, install_count) - }) - test_that("it can install on a package version", { install_count <<- 0 expect_equal(0, install_count) - expect_true(packages("robertzk/Ramd@v0.3 dplyr@1.0 glmnet@abc123")) + expect_true(packages("robertzk/Ramd@v0.3", "dplyr@1.0", "glmnet@abc123")) expect_equal(3, install_count) }) @@ -76,7 +69,7 @@ with_mock( test_that("it installs uniquely", { install_count <<- 0 expect_equal(0, install_count) - expect_true(packages("glmnet glmnet dplyr dplyr")) + expect_true(packages("glmnet", "glmnet", "dplyr", "dplyr")) expect_equal(2, install_count) }) }) From a5c30b19752ca0f5043bc0704f132f519fb2c72e Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 13:18:20 -0600 Subject: [PATCH 20/21] Prove that v in name doesn't break --- tests/testthat/test-load_pacakge.r | 36 +++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r index 7e71fe9..834b460 100644 --- a/tests/testthat/test-load_pacakge.r +++ b/tests/testthat/test-load_pacakge.r @@ -60,14 +60,34 @@ describe("version mismatching", { describe("is_version_mismatch helper function", { with_mock( `utils::packageVersion` = function(name) { package_version("1.1") }, { - expect_true(is_version_mismatch("robertzk/Ramd@v1.0")) - expect_true(is_version_mismatch("robertzk/Ramd@v1.2")) - expect_true(is_version_mismatch("robertzk/Ramd@v1.1.3")) - expect_true(is_version_mismatch("robertzk/Ramd@1.0")) - expect_false(is_version_mismatch("robertzk/Ramd@v1.1")) - expect_false(is_version_mismatch("robertzk/Ramd@1.1")) - expect_false(is_version_mismatch("robertzk/Ramd")) - expect_false(is_version_mismatch("Ramd")) + test_that("it works on lower versions", { + expect_true(is_version_mismatch("robertzk/Ramd@v1.0")) + expect_true(is_version_mismatch("robertzk/Ramd@1.0")) + expect_true(is_version_mismatch("robertzk/Ramd@0.9")) + }) + test_that("it works on higher versions", { + expect_true(is_version_mismatch("robertzk/Ramd@v1.2")) + expect_true(is_version_mismatch("robertzk/Ramd@v1.1.3")) + }) + test_that("it matches on exact versions", { + expect_false(is_version_mismatch("robertzk/Ramd@v1.1")) + expect_false(is_version_mismatch("robertzk/Ramd@1.1")) + }) + test_that("no mismatch if no version is supplied", { + expect_false(is_version_mismatch("robertzk/Ramd")) + expect_false(is_version_mismatch("Ramd")) + }) + test_that("it works if the author's name has a v", { + expect_true(is_version_mismatch("ihaveavinmyname/Ramd@1.2")) + expect_true(is_version_mismatch("ihaveavinmyname/Ramd@v1.2")) + expect_true(is_version_mismatch("ihaveavinmyname/Ramd@v1.1.3")) + expect_true(is_version_mismatch("ihaveavinmyname/Ramd@v1.0")) + expect_false(is_version_mismatch("ihaveavinmyname/Ramd@v1.1")) + expect_true(is_version_mismatch("ihaveavinmyname/andavinmypackage@v1.0")) + expect_false(is_version_mismatch("ihaveavinmyname/andavinmypackage@v1.1")) + expect_true(is_version_mismatch("ihaveavinmyname/andavinmypackage@v1.2")) + expect_false(is_version_mismatch("ihaveavinmyname/Ramd")) + }) } ) }) From 259772aff2f23c668c8801263689401f66f9496f Mon Sep 17 00:00:00 2001 From: Peter Hurford Date: Wed, 20 Jan 2016 13:25:43 -0600 Subject: [PATCH 21/21] Fix problem with branches with versions; add test --- R/load_package.r | 2 +- tests/testthat/test-load_pacakge.r | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/R/load_package.r b/R/load_package.r index e850015..1eee147 100644 --- a/R/load_package.r +++ b/R/load_package.r @@ -80,7 +80,7 @@ is_github_package <- function(name) { is_version_mismatch <- function(name) { is_versionable <- function(name) { grepl("@", name, fixed = TRUE) && - grepl(".", name, fixed = TRUE) + grepl("^[v0-9.]*$", get_version_from_ref(name)) } get_version_from_ref <- function(name) { # extract 0.3 from robertzk/Ramd@v0.3 diff --git a/tests/testthat/test-load_pacakge.r b/tests/testthat/test-load_pacakge.r index 834b460..ebe9dcf 100644 --- a/tests/testthat/test-load_pacakge.r +++ b/tests/testthat/test-load_pacakge.r @@ -77,6 +77,10 @@ describe("version mismatching", { expect_false(is_version_mismatch("robertzk/Ramd")) expect_false(is_version_mismatch("Ramd")) }) + test_that("no mismatches for refs or branches", { + expect_false(is_version_mismatch("robertzk/Ramd@1.1-thebranch")) + expect_false(is_version_mismatch("Ramd@vrefefrefref")) + }) test_that("it works if the author's name has a v", { expect_true(is_version_mismatch("ihaveavinmyname/Ramd@1.2")) expect_true(is_version_mismatch("ihaveavinmyname/Ramd@v1.2"))