From 3e186f4f31b42925193281972dab959b65316064 Mon Sep 17 00:00:00 2001 From: Alex Bokov Date: Mon, 23 Sep 2019 09:05:40 -0500 Subject: [PATCH 1/2] Added dynamic argument checking for 'readODS:read_ods()' as proposed in #223. Also more detailed tests. --- R/import_methods.R | 24 +++++++++++++++++++++++- tests/testthat/test_format_ods.R | 14 +++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/R/import_methods.R b/R/import_methods.R index 83c96bb..a6fb2d5 100644 --- a/R/import_methods.R +++ b/R/import_methods.R @@ -355,7 +355,29 @@ function(file, #' @export .import.rio_ods <- function(file, which = 1, header = TRUE, ...) { requireNamespace("readODS") - readODS::read_ods(path = file, sheet = which, col_names = header, ...) + "read_ods" <- readODS::read_ods + a <- list(...) + if ("sheet" %in% names(a)) { + which <- a[["sheet"]] + a[["sheet"]] <- NULL + } + if ("col_names" %in% names(a)) { + header <- a[["col_names"]] + a[["col_names"]] <- NULL + } + frml <- formals(readODS::read_ods) + 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 for read_ods:", + "\n", paste(unused, collapse = ', ')) + } + a <- a[intersect(names(a), names(frml))] + do.call("read_ods" + ,c(list(path = file, sheet = which, col_names = header),a)) } #' @importFrom utils type.convert diff --git a/tests/testthat/test_format_ods.R b/tests/testthat/test_format_ods.R index 3df758f..46f9fc9 100644 --- a/tests/testthat/test_format_ods.R +++ b/tests/testthat/test_format_ods.R @@ -3,10 +3,22 @@ require("datasets") test_that("Import from ODS", { skip_if_not_installed(pkg="readODS") - ods <- import(system.file("examples", "mtcars.ods", package = "rio")) + expect_message(ods0 <- import(system.file("examples", "mtcars.ods", + package = "rio")), + 'Parsed with column specification:', + label = "ODS import with default arguments works") + expect_warning(ods <- import(system.file("examples", "mtcars.ods" + , package = "rio"), + sheet = 1, col_names = TRUE, + path = 'ignored value', + invalid_argument = 42), + "The following arguments were ignored for read_ods:\ninvalid_argument, path", + label = "ODS import ignores redundant and unknown arguments with a warning") + expect_identical(ods0,ods, label = "ODS import ignored arguments don't affect output") expect_true(is.data.frame(ods), label = "ODS import returns data.frame") expect_true(identical(names(mtcars), names(ods)), label = "ODS import returns correct names") expect_true(identical(dim(mtcars), dim(ods)), label = "ODS import returns correct dimensions") + expect_equivalent(ods,mtcars,label = "ODS import returns correct values") }) test_that("Export to ODS", { From a39354c8a8f8b22284415996b95c8e9b1197c9a9 Mon Sep 17 00:00:00 2001 From: Alex Bokov Date: Wed, 2 Oct 2019 09:39:27 -0500 Subject: [PATCH 2/2] Fixed coding style issues from #225#pullrequestreview-296157799 in import_methods.R --- R/import_methods.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/import_methods.R b/R/import_methods.R index a6fb2d5..d2c4908 100644 --- a/R/import_methods.R +++ b/R/import_methods.R @@ -372,12 +372,12 @@ function(file, a[["path"]] <- NULL } if (length(unused)>0) { - warning("The following arguments were ignored for read_ods:", - "\n", paste(unused, collapse = ', ')) + warning("The following arguments were ignored for read_ods:\n", + paste(unused, collapse = ', ')) } a <- a[intersect(names(a), names(frml))] - do.call("read_ods" - ,c(list(path = file, sheet = which, col_names = header),a)) + do.call("read_ods", + c(list(path = file, sheet = which, col_names = header),a)) } #' @importFrom utils type.convert