Skip to content

Commit

Permalink
Enable an option for backward compatibility
Browse files Browse the repository at this point in the history
  • Loading branch information
@chainsawriot
chainsawriot committed Sep 8, 2023
1 parent e00564a commit 1813764
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 36 deletions.
22 changes: 18 additions & 4 deletions R/read_ods.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@
}

.change_df_with_col_row_header <- function(x, col_header, row_header, .name_repair) {
if (nrow(x) == 1 && col_header) {
if (nrow(x) == 1 && col_header && isFALSE(getOption("readODS.v200", FALSE))) {
return(.return_zerorow(x, row_header, .name_repair))
}
if (((nrow(x) < 2 && col_header ) || (ncol(x) < 2 && row_header)) &&
isTRUE(getOption("readODS.v200", FALSE))) {
## 2.0.0 behavior
warning("Cannot make column/row names if this would cause the dataframe to be empty.", call. = FALSE)
return(x)
}
irow <- ifelse(col_header, 2, 1)
jcol <- ifelse(row_header, 2, 1)

Expand Down Expand Up @@ -115,8 +121,10 @@
}

.return_empty <- function(as_tibble = FALSE) {
warning("empty sheet, return empty data frame.", call. = FALSE)
if(as_tibble) {
if (getOption("readODS.v200", FALSE)) {
warning("empty sheet, return empty data frame.", call. = FALSE)
}
if (as_tibble) {
return(tibble::tibble())
}
return(data.frame())
Expand Down Expand Up @@ -210,7 +218,13 @@
stop_col = limits["max_col"],
sheet = sheet,
formula_as_formula = formula_as_formula)
if ((strings[1] == 0 || strings[2] == 0) || (strings[1] == 1 && row_names)) {

if (((strings[1] == 0 || strings[2] == 0)) &&
isTRUE(getOption("readODS.v200", FALSE))) {
return(.return_empty(as_tibble = as_tibble))
}
if (((strings[1] == 0 || strings[2] == 0) || (strings[1] == 1 && row_names)) &&
isFALSE(getOption("readODS.v200", FALSE))) {
return(.return_empty(as_tibble = as_tibble))
}
res <- as.data.frame(
Expand Down
24 changes: 19 additions & 5 deletions tests/testthat/test_read_fods.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,29 @@ test_that("Error when not correct", {
expect_error(read_fods("../testdata/notreal.ods"))
})

test_that("Return blank/error if mangled FODS", {
expect_warning(read_fods("../testdata/norows.fods"))
expect_warning(read_fods("../testdata/nocells.fods"))
})

test_that("fix #157", {
skip_if(file.access("~/", 2) != 0)
file.copy("../testdata/flat.fods", "~/flat_you_must_not_use_this_path.fods")
expect_error(read_fods("~/flat_you_must_not_use_this_path.fods"), NA)
expect_error(read_fods("~/flat_you_must_not_use_this_path_not.fods"))
on.exit(unlink("~/flat_you_must_not_use_this_path.fods"))
})

## default

test_that("Return blank/error if mangled FODS", {
expect_silent(read_fods("../testdata/norows.fods"))
expect_silent(read_fods("../testdata/nocells.fods"))
})

## V2.0.0 behavior: backward compatibility

ori_option <- getOption("readODS.v200") ## probably NULL
options("readODS.v200" = TRUE)

test_that("Return blank/error if mangled FODS v2.0.0", {
expect_warning(read_fods("../testdata/norows.fods"))
expect_warning(read_fods("../testdata/nocells.fods"))
})

options("readODS.v200" = ori_option)
88 changes: 61 additions & 27 deletions tests/testthat/test_read_ods.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,6 @@ test_that("Incorrect Argument", {
expect_error(read_ods(path = "../testdata/sum.ods", row_names = TRUE), "Tibbles do not support")
})

test_that("Single column ODS", {
single_col <- read_ods('../testdata/sum.ods', sheet = 1)
expect_equal(ncol(single_col),1)
expect_equal(colnames(single_col), c("X1"))
##expect_warning(read_ods('../testdata/sum.ods', sheet = 1, row_names = TRUE, as_tibble = FALSE), "Cannot make")
})

test_that("Single row ODS", {
##expect_warning(single_row <- read_ods('../testdata/onerow.ods', sheet = 1), "Cannot make")
##expect_equal(nrow(single_row), 1)
##expect_equal(single_row[[1,1]], 1)
})

test_that("Single column range", {
expect_error(read_ods("../testdata/starwars.ods", range="A1:A5"), NA)
})
Expand Down Expand Up @@ -51,15 +38,6 @@ test_that("eating space issue #74", {
expect_equal(df[[1,1]], "A B\nC")
})

test_that("skip", {
expect_silent(x <- read_ods("../testdata/starwars.ods", skip = 0))
expect_equal(nrow(x), 10)
expect_message(x <- read_ods("../testdata/starwars.ods", skip = 1, col_names = FALSE))
expect_equal(nrow(x), 10)
expect_warning(x <- read_ods("../testdata/starwars.ods", skip = 11), "empty sheet")
expect_equal(nrow(x), 0)
})

test_that("Check .name_repair works properly", {
expect_silent(x <- read_ods("../testdata/test_naming.ods", .name_repair = "minimal"))
expect_equal(colnames(x), c("a", "a.1", "Var.3"))
Expand All @@ -84,11 +62,6 @@ test_that("Deals with repeated spaces correctly when fetching only part of sheet
expect_equal(read_ods("../testdata/excel_repeat.ods", range = "A9:B18", col_names = FALSE)[[5,1]], "C")
})

test_that("Warns of empty sheet", {
expect_warning(read_ods("../testdata/empty.ods"))
expect_warning(read_fods("../testdata/empty.fods"))
})

test_that("read with column headers", {
expect_silent(x <- read_ods("../testdata/starwars.ods", col_names = TRUE))
expect_equal(ncol(x), 3)
Expand All @@ -97,3 +70,64 @@ test_that("read with column headers", {
expect_equal(ncol(x), 2)
expect_equal(colnames(x), c("homeworld", "species"))
})

## default single behavior
test_that("Single column ODS", {
single_col <- read_ods('../testdata/sum.ods', sheet = 1)
expect_equal(ncol(single_col),1)
expect_equal(colnames(single_col), c("X1"))
expect_equal(nrow(read_ods('../testdata/sum.ods', sheet = 1, row_names = TRUE, as_tibble = FALSE)), 0)
})

test_that("Single row ODS", {
expect_silent(single_row <- read_ods('../testdata/onerow.ods', sheet = 1))
expect_equal(nrow(single_row), 0)
})

test_that("skip", {
expect_silent(x <- read_ods("../testdata/starwars.ods", skip = 0))
expect_equal(nrow(x), 10)
expect_message(x <- read_ods("../testdata/starwars.ods", skip = 1, col_names = FALSE))
expect_equal(nrow(x), 10)
expect_silent(x <- read_ods("../testdata/starwars.ods", skip = 11))
expect_equal(nrow(x), 0)
})

test_that("No Warning of empty sheet", {
expect_silent(read_ods("../testdata/empty.ods"))
expect_silent(read_fods("../testdata/empty.fods"))
})

## V2.0.0 behavior: backward compatibility

ori_option <- getOption("readODS.v200") ## probably NULL
options("readODS.v200" = TRUE)

test_that("Single column ODS v2.0.0", {
single_col <- read_ods('../testdata/sum.ods', sheet = 1)
expect_equal(ncol(single_col),1)
expect_equal(colnames(single_col), c("X1"))
expect_warning(read_ods('../testdata/sum.ods', sheet = 1, row_names = TRUE, as_tibble = FALSE), "Cannot make")
})

test_that("Single row ODS v2.0.0", {
expect_warning(single_row <- read_ods('../testdata/onerow.ods', sheet = 1), "Cannot make")
expect_equal(nrow(single_row), 1)
expect_equal(single_row[[1,1]], 1)
})

test_that("skip v2.0.0", {
expect_silent(x <- read_ods("../testdata/starwars.ods", skip = 0))
expect_equal(nrow(x), 10)
expect_message(x <- read_ods("../testdata/starwars.ods", skip = 1, col_names = FALSE))
expect_equal(nrow(x), 10)
expect_warning(x <- read_ods("../testdata/starwars.ods", skip = 11), "empty sheet")
expect_equal(nrow(x), 0)
})

test_that("Warns of empty sheet", {
expect_warning(read_ods("../testdata/empty.ods"))
expect_warning(read_fods("../testdata/empty.fods"))
})

options("readODS.v200" = ori_option)

0 comments on commit 1813764

Please sign in to comment.