read_ functions: control assert_that messages in case of error

[cecileherr]

I got errors with vage formulation such as:

Error: assert_that: length of assertion is not 1

It could be useful to check when this type of errors occurs and to add a "msg = " argument when needed

Here is a (probably uncomplete) list of arguments for which the standard error messages should be checked:

• file.exists
• is.string
• is.flag
• noNA

[cecileherr]

The following error messages are ok and do not need further explanation:

assert_that(file.exists(file))

> read_habitatstreams(file = file.path(fileman_up("n2khab_data"), "10_raw/habitatstreams2"))
 Error: Path 'C:\Users\cecile_herr\GitHubRepos\n2khab_data/10_raw/habitatstreams2' does not exist

> read_habitatstreams(file = NA)
 Error in file.exists(file) : invalid 'file' argument

assert_that(is.flag(source_text), noNA(source_text))

 > read_habitatstreams(source_text = "rr")
 Error: source_text is not a flag (a length one logical vector). 

> read_habitatstreams(source_text = NA)
 Error: source_text contains 1 missing values  

assert_that(is.string(version))

> read_habitatmap_stdized(version = 2)
 Error: version is not a string (a length one character vector).

[cecileherr]

Obscure errors occur when the folder "n2khab_data" is not found, that is when fileman_up("n2khab_data") returns NULL (typically when I try to use the n2khab layers from a -temporary- script om my desktop for a quick check.

Examples:

> habitatmap <- read_habitatmap()
 Error: assert_that: length of assertion is not 1 

> habitatmap_st <- read_habitatmap_stdized()
 Error in if (nchar(dsn) < 1) { : argument is of length zer

[cecileherr]

Could this be a solution? (an example with read_watersurfaces)

Replace this:

read_watersurfaces <-
    function(file = file.path(fileman_up("n2khab_data"),
                              "10_raw/watersurfaces"),
             extended = FALSE,
             version = "watersurfaces_v1.0") {

        assert_that(file.exists(file))
        assert_that(is.string(version))

(...)

}

by this:

read_watersurfaces <-
    function(file = NULL,
             extended = FALSE,
             version = "watersurfaces_v1.0") {

        if (is.null(file)) {

            if (is.null(fileman_up("n2khab_data"))) {
                stop("n2khab_data folder not found")
            } else {
                file = file.path(fileman_up("n2khab_data"),
                              "10_raw/watersurfaces"  
            }

        } else {
            assert_that(file.exists(file))  
            }

        assert_that(is.string(version))

(...)

}

[florisvdh]

Thanks for having a look. First note that the <- function(............) part is used by roxygen2 to generate the documentation, notably the usage section. Therefore I suggest to keep showing actual defaults to the user if possible.

So I'd keep the default, but in order to get better behaviour, you could update fileman_up() to not return NULL if name is not found (lines below) but instead error with stop(name, " was not found. Searched up to ", path). What do you think?

https://github.com/inbo/n2khab/blob/a7c450a9198180e06e340c30443d94111e4584c8/R/filemanagement.R#L373-L378

Then, if n2khab_data is found but file is not present, error message should be something like:

> assertthat::assert_that(file.exists(file = file.path(fileman_up("n2khab_data"),
+                                                      "10_raw/habitatstreamsies")))
Error: Path '/media/floris/....../n2khab_data/10_raw/habitatstreamsies' does not exist

NULL IMO is more appropriate for optional arguments with no obvious default, e.g. if the argument only triggers an (extra) action when user assigns a value to the argument. Then having a default NULL in the usage section still makes clear it is not a required argument (in contrast to no default at all). Compare with https://adv-r.hadley.nz/functions.html#missing-arguments.

[cecileherr]

Yes, you are right, I was overlooking roxygen2. A change in fileman_up() seems the way to go.

Shall I implement this change (while I am busy) or do you prefer to do it yourself?

[florisvdh]

Shall I implement this change (while I am busy)

Yes, please.

[florisvdh]

Solved by #114.