Remove unnecessary requireNamespace() from internal code

[sda030]

Hi
requireNamespace() only returns TRUE or FALSE, depending upon whether the package is installed. However, as it is currently used in the .import-methods, they serve no purpose. The imported packages should be loaded into the namespace with the ::. And now it is slightly strange to have the message upon import(), as one of the reasons for using rio is to avoid having to load all the various upstream packages.

>import("testfile.parquet")
Loading required namespace: arrow
>

[chainsawriot]

@sda030 Thank you for reporting this. I agree with you that this is quite counterproductive to load the entire namespace if a package is installed.

I think the intended purpose of the requireNamespace() should be like this:

.check_install <- function(pkg) {
    if (identical(find.package(pkg, quiet = TRUE), character(0))) {
        stop("Please install this package: ", pkg, call. = FALSE)
    }
    return(invisible(NULL))
}

.import.rio_parquet <- function(file, which = 1, as_data_frame = TRUE, ...) {
    .check_install("arrow")
    ## the as_data_frame = TRUE is kind of problematic too; probably as_data_frame = as_data_frame
    arrow::read_parquet(file = file, as_data_frame = TRUE, ...) 
}

.import.rio_parquet_ori <- function(file, which = 1, as_data_frame = TRUE, ...) {
    requireNamespace("arrow")
    arrow::read_parquet(file = file, as_data_frame = TRUE, ...) 
}

.import.rio_parquet("~/dev/misc/iris.parquet")
#> Error: Please install this package: arrow

.import.rio_parquet_ori("~/dev/misc/iris.parquet")
#> Loading required namespace: arrow
#> Error in loadNamespace(x): there is no package called 'arrow'

^{Created on 2023-08-29 with reprex v2.0.2}

Does the first one look like a reasonable solution to you?

[sda030]

Hi @chainsawriot, have you read this article by Christian Hohenfeld regarding checking if a package is installed? Seems most approaches are not perfect. Could test out various approaches and profile it with regard to memory allocation and speed?

And yes, I agree regarding the as_data_frame argument.