Fix #151 (#182)

* Fix #151

* Cover the uncovered line

* Refactor `.determine_ods_format` to be reused in `list_ods_sheets`

* Cover the uncovered line

* Make `list_ods_sheets` a common interface

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: readODS
 Type: Package
 Title: Read and Write ODS Files
-Version: 2.1.1
+Version: 2.2.0
 Authors@R: 
     c(person("Gerrit-Jan", "Schutten", role = c("aut"), email = "[email protected]"),
     person("Chung-hong", "Chan", role = c("aut", "cre"), email = "[email protected]", comment = c(ORCID = "0000-0002-6232-7530")),
@@ -29,7 +29,8 @@ Imports:
     stringi,
     tibble,
     vctrs (>= 0.4.2),
-    zip
+    zip,
+    tools
 LinkingTo: 
     cpp11 (>= 0.4.6)
 Suggests:

NEWS.md

@@ -1,3 +1,7 @@
+# readODS 2.2.0
+
+* Fix #151 - Now `read_ods()` and `list_ods_sheets()` can also be used to process flat ods files. `read_fods()` and `list_fods_sheets()` are still available, but not as the so-called "common interface."
+
 # readODS 2.1.1
 
 ## Bug fixes

R/list_ods_sheets.R

@@ -7,24 +7,31 @@
 
 #' Get information in an (F)ODS File
 #'
-#' `list_(f)ods_sheets` lists all sheets in an (f)ods file.
+#' `list_ods_sheets` lists all sheets in an (f)ods file. The function can be used for listing sheets in both ods and flat ods files. (\code{list_fods_sheets}) is also available, which can only list sheets in flat ods files.
 #'
 #' @param path Path to the (f)ods file
 #' @param include_external_data A boolean value to show or hide sheets containing archived linked data (default false)
 #' @return A character vector of sheet names
 #' @details The default "include_external_data" for `ods_sheets` is TRUE to maintain compatibility with version 1 of readODS. It will change to `TRUE` in version 3.
 #' @author Peter Brohan <peter.brohan+cran@@gmail.com>, Chung-hong Chan <chainsawtiney@@gmail.com>, Gerrit-Jan Schutten <phonixor@@gmail.com>
+#' @inheritParams read_ods
 #' @examples
 #' \dontrun{
 #' # Get the list of names of sheets
 #' list_ods_sheets("starwars.ods")
+#' list_ods_sheets("starwars.fods")
+#' # Using list_fods_sheets, although you don't have to
 #' list_fods_sheets("starwars.fods")
 #' }
 #' @seealso
 #' use \code{\link{read_ods}} to read the data
 #' @export
-list_ods_sheets <- function(path, include_external_data = FALSE) {
-    .list_ods_sheets(path, include_external_data = include_external_data, .list_function = get_sheet_names_)
+list_ods_sheets <- function(path, include_external_data = FALSE, ods_format = c("auto", "ods", "fods"), guess = FALSE) {
+    ods_format <- .determine_ods_format(path = path, guess = guess, ods_format = match.arg(ods_format))
+    if (ods_format == "ods") {
+        return(.list_ods_sheets(path, include_external_data = include_external_data, .list_function = get_sheet_names_))
+    }
+    .list_ods_sheets(path, include_external_data = include_external_data, .list_function = get_flat_sheet_names_)
 }
 
 #' @rdname list_ods_sheets

R/read_ods.R

@@ -83,9 +83,6 @@
                         strings_as_factors = FALSE,
                         verbose = FALSE,
                         as_tibble = TRUE) {
-    if (missing(path) || !is.character(path)) {
-        stop("No file path was provided for the 'path' argument. Please provide a path to a file to import.", call. = FALSE)
-    }
     if (!file.exists(path)) {
         stop("file does not exist", call. = FALSE)
     }
@@ -246,10 +243,39 @@
     return(res)
 }
 
+.determine_ods_format <- function(path, guess = FALSE, ods_format = "auto") {
+    if (missing(path) || !is.character(path)) {
+        stop("No file path was provided for the 'path' argument. Please provide a path to a file to import.", call. = FALSE)
+    }
+    if (ods_format != "auto") {
+        return(ods_format)
+    }
+    ext <- tolower(tools::file_ext(path))
+    formats <- c(
+        ods = "ods",
+        fods = "fods",
+        xml = "fods"
+    )
+    if (!isTRUE(guess)) {
+        ext <- unname(formats[ext])
+        if (is.na(ext)) {
+            return("ods")
+        }
+        return(ext)
+    }
+    zip_sig <- as.raw(c(
+        "0x50", "0x4B", "0x03", "0x04"
+    ))
+    if (identical(zip_sig, readBin(path, n = 4, what = "raw"))) {
+        return("ods")
+    }
+    return("fods")
+}
+
 #' Read Data From (F)ODS File
 #'
-#' read_ods is a function to read a single sheet from an (f)ods file and return a data frame. For flat ods files (.fods or .xml),
-#' use (\code{read_fods}).
+#' read_ods is a function to read a single sheet from an (f)ods file and return a data frame. The function can be used for reading both ods and flat ods files.
+#' (\code{read_fods}) is also available, which can only read flat ods files.
 #'
 #' @param path path to the (f)ods file.
 #' @param sheet sheet to read. Either a string (the sheet name), or an integer sheet number. The default is 1.
@@ -273,6 +299,10 @@
 #'
 #'  Default is `"unique"`.
 #'
+#' @param ods_format character, must be "auto", "ods" or "fods". The default "auto" is to determine the format automatically. By default, the format is determined by file extension, unless `guess` is `FALSE`.
+#' @param guess logical. If the file extension is absent or not recognized, this
+#'   controls whether we attempt to guess format based on the file signature or
+#'   "magic number".
 #' @return A tibble (\code{tibble}) or data frame (\code{data.frame}) containing a representation of data in the (f)ods file.
 #' @author Peter Brohan <peter.brohan+cran@@gmail.com>, Chung-hong Chan <chainsawtiney@@gmail.com>, Gerrit-Jan Schutten <phonixor@@gmail.com>
 #' @examples
@@ -284,17 +314,19 @@
 #' # Read a specific range, e.g. A1:C11
 #' read_ods("starwars.ods", sheet = 2, range = "A1:C11")
 #' # Read an FODS file
-#' read_fods("starwars.fods")
+#' read_ods("starwars.fods")
 #' # Read a specific sheet, e.g. the 2nd sheet
-#' read_fods("starwars.fods", sheet = 2)
+#' read_ods("starwars.fods", sheet = 2)
 #' # Read a specific range, e.g. A1:C11
-#' read_fods("starwars.fods", sheet = 2, range = "A1:C11")
+#' read_ods("starwars.fods", sheet = 2, range = "A1:C11")
 #' # Give a warning and read from Sheet1 (not 2)
-#' read_fods("starwars.fods", sheet = 2, range = "Sheet1!A1:C11")
+#' read_ods("starwars.fods", sheet = 2, range = "Sheet1!A1:C11")
 #' # Specifying col_types as shorthand, the third column as factor; other by guessing
 #' read_ods("starwars.ods", col_types = "??f")
 #' # Specifying col_types as list
 #' read_ods("starwars.ods", col_types = list(species = "f"))
+#' # Using read_fods, although you don't have to
+#' read_ods("starwars.fods")
 #' }
 #' @export
 read_ods <- function(path,
@@ -309,7 +341,10 @@ read_ods <- function(path,
                      strings_as_factors = FALSE,
                      verbose = FALSE,
                      as_tibble = TRUE,
-                     .name_repair = "unique") {
+                     .name_repair = "unique",
+                     ods_format = c("auto", "ods", "fods"),
+                     guess = FALSE) {
+    ods_format <- .determine_ods_format(path = path, guess = guess, ods_format = match.arg(ods_format))
     ## Should use match.call but there's a weird bug if one of the variable names is 'file'
     .read_ods(path = path,
         sheet = sheet,
@@ -324,7 +359,7 @@ read_ods <- function(path,
         verbose = verbose,
         as_tibble = as_tibble,
         .name_repair = .name_repair,
-        flat = FALSE)
+        flat = ods_format == "fods")
 }
 
 #' @rdname read_ods

tests/testthat/test_determine.R

@@ -0,0 +1,33 @@
+test_that(".determine_ods_format works", {
+    expect_equal(.determine_ods_format(readODS::write_ods(iris)), "ods")
+    expect_equal(.determine_ods_format(readODS::write_fods(iris)), "fods")
+    expect_equal(.determine_ods_format(readODS::write_ods(iris, tempfile(fileext = ".fods"))), "fods")
+    expect_equal(.determine_ods_format(readODS::write_ods(iris, tempfile(fileext = ".fods")), guess = TRUE), "ods")
+    expect_equal(.determine_ods_format(readODS::write_fods(iris, tempfile(fileext = ".xml"))), "fods")
+    expect_equal(.determine_ods_format(readODS::write_ods(iris, tempfile(fileext = ".fods")), guess = TRUE), "ods")
+    expect_equal(.determine_ods_format(readODS::write_ods(iris, tempfile(fileext = ".xml"))), "fods")
+    expect_equal(.determine_ods_format(readODS::write_ods(iris, tempfile(fileext = ".xml")), guess = TRUE), "ods")
+    expect_equal(.determine_ods_format(readODS::write_fods(iris, tempfile(fileext = ".ods")), guess = TRUE), "fods")
+    ## don't guess
+    expect_equal(.determine_ods_format(readODS::write_fods(iris, tempfile(fileext = ".ods")), ods_format = "zip"), "zip")
+})
+
+test_that("integration in read_ods", {
+    expect_error(readODS::read_ods(readODS::write_fods(iris)), NA)
+    expect_error(readODS::read_ods(readODS::write_ods(iris)), NA)
+    expect_error(readODS::read_fods(readODS::write_ods(iris))) ## err, read_fods is not a common interface
+    expect_error(readODS::read_fods(readODS::write_fods(iris)), NA)
+    ## override
+    temp_fods <- tempfile(fileext = ".fods")
+    expect_error(readODS::read_ods(readODS::write_ods(iris, temp_fods), ods_format = "ods"), NA)
+    expect_error(readODS::read_ods(readODS::write_ods(iris, temp_fods), guess = TRUE), NA)
+})
+
+test_that("integration in list_ods_sheets", {
+    expect_error(readODS::list_ods_sheets(readODS::write_fods(iris)), NA)
+    expect_error(readODS::list_ods_sheets(readODS::write_ods(iris)), NA)
+    expect_error(readODS::list_fods_sheets(readODS::write_ods(iris))) ## err
+    temp_fods <- tempfile(fileext = ".fods")
+    expect_error(readODS::list_ods_sheets(readODS::write_ods(iris, temp_fods), ods_format = "ods"), NA)
+    expect_error(readODS::list_ods_sheets(readODS::write_ods(iris, temp_fods), guess = TRUE), NA)
+})

vignettes/overview.Rmd

@@ -95,22 +95,22 @@ read_ods("plant_multi.ods", sheet = "plant")
 
 ## Flat ODS files (`.xml` or `.fods`)
 
-Can be read with `read_fods()` (note that the same function is used to read flat files, no matter the extension).
+Can be read with `read_ods()` [^1] (note that the same function is used to read flat files, no matter the extension).
 This has the same behaviour and arguments as `read_ods()`
 
 ```{r read fods, eval = file.exists("plant.fods")}
 read_fods("plant.fods")
 ```
 
-`write_fods()` can be used to write Flat ODS files
+`write_ods()` can be used to write Flat ODS files
 
 ```{r write_fods}
-write_fods(PlantGrowth, "plant.fods")
+write_ods(PlantGrowth, "plant.fods")
 ```
 
 ## Misc.
 
-Use the function `list_ods_sheets()` or `list_fods_sheets()` to list out all sheets in an (F)ODS file.
+Use the function `list_ods_sheets()` to list out all sheets in an (F)ODS file.
 
 ```{r, list_ods_sheets}
 list_ods_sheets("plant.ods")
@@ -186,3 +186,7 @@ unlink("plant.ods")
 unlink("plant.fods")
 unlink("plant_multi.ods")
 ```
+
+---
+
+[^1]: `read_fods()` and `list_fods_sheets()` are also available. But since version 2.2.0 `read_ods()` and `list_ods_sheets()` can determine whether the file at the `path` argument is flat or not.