'.import.rio_xls()' no longer passing '...' to 'read_xls()' (gesistsa…

…#230) * '.import.rio_xls()' no longer passing '...' to 'read_xls()' because that functions cannot accept '...' arguments. Instead, the length of '...' is checked and if > 0 a warning is issued. With corresponding tests. Also went back and changed my earlier 'read_xlsx()' test to be 'expect_warning()' instead of 'expect_true()' since by that point that's what needs testing.
bokov · Oct 3, 2019 · b9f78fa · b9f78fa
1 parent 6dce742
commit b9f78fa
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 11 deletions.
diff --git a/R/import_methods.R b/R/import_methods.R
@@ -301,17 +301,23 @@ function(file,
 #' @export
 .import.rio_xls <- function(file, which = 1, ...) {
 
-    Call <- match.call(expand.dots = TRUE)
-    if ("which" %in% names(Call)) {
-        Call$sheet <- Call$which
-        Call$which <- NULL
+    a <- list(...)
+    if ("sheet" %in% names(a)) {
+        which <- a[["sheet"]]
+        a[["sheet"]] <- NULL
     }
-
-    Call$path <- file
-    Call$file <- NULL
-    Call$readxl <- NULL
-    Call[[1L]] <- as.name("read_xls")
-    eval.parent(Call)
+    frml <- formals(read_xls)
+    unused <- setdiff(names(a), names(frml))
+    if ("path" %in% names(a)) {
+        unused <- c(unused, 'path')
+        a[["path"]] <- NULL
+    }
+    if (length(unused)>0) {
+        warning("The following arguments were ignored by read_xls:\n",
+                paste(unused, collapse=', '))
+    }
+    a <- a[intersect(names(a), names(frml))]
+    do.call("read_xls", c(list(path = file, sheet = which), a))
 }
 
 #' @importFrom readxl read_xlsx

diff --git a/tests/testthat/test_format_xls.R b/tests/testthat/test_format_xls.R
@@ -12,7 +12,9 @@ test_that("Import from Excel (.xlsx)", {
     expect_true(is.data.frame(import("iris.xlsx", sheet = 1)))
     expect_true(is.data.frame(import("iris.xlsx", which = 1)))
     expect_true(nrow(import("iris.xlsx", n_max = 42))==42)
-    expect_true(is.data.frame(import("iris.xlsx", nrows = 42)))
+    expect_warning(is.data.frame(import("iris.xlsx", nrows = 42)),
+                   "The following arguments were ignored when readxl = TRUE:\nnrows",
+                   label="xlsx reads the file and ignores unused arguments with warning")
 })
 
 test_that("Import from Excel (.xls)", {
@@ -22,6 +24,11 @@ test_that("Import from Excel (.xls)", {
                                                  package='rio'), sheet = 1)))
     expect_true(is.data.frame(import(system.file('examples','iris.xls',
                                                  package='rio'), which = 1)))
+    expect_warning(is.data.frame(import(system.file('examples','iris.xls',
+                                                 package='rio'), which = 1,
+                                        invalid_argument = 42)),
+                                 "The following arguments were ignored by read_xls:",
+                                 label="xls reads the file and ignores unused arguments with warning")
 })