Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring of check_package() + return TRUE instead of package + tests #247

Merged
merged 43 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
3f85f5b
move `check_package()` outside of zzz.R
PietrH Jul 14, 2023
e871c25
rename multimedia to media
PietrH Jul 14, 2023
06e3bfd
add comment on deprecation warning
PietrH Jul 14, 2023
e16686d
When a package is valid, return TRUE
PietrH Jul 14, 2023
4449ff6
test deprecation warning
PietrH Jul 14, 2023
ed42577
test package class
PietrH Jul 14, 2023
a541ff5
test for missing data elements
PietrH Jul 14, 2023
2121af8
test for number/identity of data elements
PietrH Jul 14, 2023
e0f2e59
add error messages
PietrH Jul 14, 2023
36871af
refactor so it works
PietrH Jul 14, 2023
81c8371
test for data.frame elements
PietrH Jul 14, 2023
9e4f5cc
test for no error if media is not imported
PietrH Jul 14, 2023
efb23d3
test for error when media is not a data.frame
PietrH Jul 14, 2023
b444abe
stylr
PietrH Jul 14, 2023
2c9e2b9
no longer assign check_package() to validate
PietrH Jul 14, 2023
8043218
silence messages during testing
PietrH Jul 14, 2023
7e6a439
remove test covered by test-check_package.R
PietrH Jul 14, 2023
390f95d
remove test covered by test-check_package.R
PietrH Jul 14, 2023
9aa6cf0
`check_package()` now returns TRUE on a valid package
PietrH Jul 14, 2023
a22bd7c
Merge branch '245-have-check_package-return-error-or-true-on-valid' o…
PietrH Jul 14, 2023
1d40a02
silence messages in testing output
PietrH Jul 14, 2023
edcccdf
Merge main by resolving conflicts
damianooldoni Jul 24, 2023
46ab17a
Adap read function to new output check_package
damianooldoni Jul 24, 2023
15d971c
Avoid returning TRUE explicitly
damianooldoni Jul 24, 2023
36404ca
Improve documentation about returned object
damianooldoni Jul 24, 2023
ada3a0d
Run devtools::document()
damianooldoni Jul 24, 2023
53133e7
Merge branch 'main' into 245-have-check_package-return-error-or-true-…
damianooldoni Jul 24, 2023
ca26d97
Add/ improve messages in assertions
damianooldoni Jul 24, 2023
0c748eb
Remove typo while forming the message
damianooldoni Jul 24, 2023
819fdce
Add return(TRUE) back at the end of function
damianooldoni Jul 24, 2023
0fbdda2
Avoid using media element while testing missing element in data
damianooldoni Jul 24, 2023
353a18c
Add test about media flag argument
damianooldoni Jul 24, 2023
366d81c
Improve test definition back
damianooldoni Jul 24, 2023
7c21229
Run devtools::document()
damianooldoni Jul 24, 2023
3e4a077
Transfer deprecated datapkg arg to package
damianooldoni Jul 24, 2023
efa4568
Add tests about deprecation datapkg
damianooldoni Jul 24, 2023
31b35e9
Add deprecation to datapkg where forgotten
damianooldoni Jul 25, 2023
a4d6a10
Apply devtools::document()
damianooldoni Jul 25, 2023
1a43639
Support deprecation in get_rai_individuals as well
damianooldoni Jul 25, 2023
5439c23
Use dplyr::func_name() syntax in test
damianooldoni Jul 25, 2023
5155d10
Add suppressMessages() to better read output tests
damianooldoni Jul 25, 2023
db8e01a
Add suppressMessages in test for read_ function
damianooldoni Jul 25, 2023
a51a396
Add tests for deprecated arg datapkg
damianooldoni Jul 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions R/check_package.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
#' Check validity camera trap data package
#'
#' Checks the validity of a camera trap data package.
#' It checks whether the data package is a list containing an element called
#' `data` with the following resources as tibble data frames:
#' - `observations`
#' - `media`
#' - `deployments`
#'
#' @param package Camera trap data package
#' @param datapkg Deprecated. Use `package` instead.
#' @param media Has the `media` resource been loaded while reading the data
#' package? Default: `FALSE`.
#' @return `TRUE` or error.
#' @noRd
check_package <- function(package = NULL,
datapkg = NULL,
function_name,
media = FALSE) {
if (lifecycle::is_present(datapkg) & !is.null(datapkg)) {
lifecycle::deprecate_warn(
when = "0.16.0",
what = paste0(function_name, "(datapkg = )"),
with = paste0(function_name, "(package = )")
)
if (is.null(package)) {
package <- datapkg
}
}
# check media arg
assertthat::assert_that(
media %in% c(TRUE, FALSE),
msg = "`media` must be a logical: TRUE or FALSE"
)
# camera trap data package is a list
assertthat::assert_that(is.list(package),
msg = "package is not a list.")
assertthat::assert_that(!is.data.frame(package),
msg = "package is not a list.")
# check existence of an element called data
assertthat::assert_that("data" %in% names(package),
msg = "data element is missing from package")
# check validity data element of package: does it contain deployments and
# observations?
elements <- c("deployments", "observations")
if (media) {
elements <- c(elements, "media")
}
tables_absent <- elements[
!elements %in% names(package$data)
]
assertthat::assert_that(
length(tables_absent) == 0,
msg = glue::glue(
"Can't find {length(tables_absent)} elements in data package: {tables_absent*}",
.transformer = collapse_transformer(sep = ", ", last = " and ")
)
)
if (media) {
assertthat::assert_that(
!is.null(package$data$media),
msg = glue::glue("Can't find media in .$data.")
)
}
# check observations and deployments are data.frames
assertthat::assert_that(is.data.frame(package$data$observations))
assertthat::assert_that(is.data.frame(package$data$deployments))
# check media is a data.frame (if imported, i.e. if not NULL)
if (!is.null(package$data$media)) {
assertthat::assert_that(is.data.frame(package$data$media))
}
# If no errors are encountered, TRUE is returned
return(TRUE)
}
3 changes: 2 additions & 1 deletion R/check_species.R
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ check_species <- function(package = NULL,
arg_name = "species",
datapkg = lifecycle::deprecated()) {
# Check camera trap data package
package <- check_package(package, datapkg, "check_species")
check_package(package, datapkg, "check_species")

assertthat::assert_that(
!is.null(species) & length(species) > 0,
msg = "`species` parameter must be specified"
Expand Down
9 changes: 6 additions & 3 deletions R/get_cam_op.R
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,13 @@ get_cam_op <- function(package = NULL,
...,
station_col = "locationName",
use_prefix = FALSE,
datapkg = NULL) {
datapkg = lifecycle::deprecated()) {
# check camera trap data package
package <- check_package(package, datapkg, "get_cam_op")

check_package(package, datapkg, "get_cam_op")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# Check that station_col is a single string
assertthat::assert_that(assertthat::is.string(station_col))
# Check that station_col is one of the columns in deployments
Expand Down
9 changes: 6 additions & 3 deletions R/get_custom_effort.R
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ get_custom_effort <- function(package = NULL,
end = NULL,
group_by = NULL,
unit = "hour",
datapkg = NULL) {
datapkg = lifecycle::deprecated()) {
# define possible unit values
units <- c("hour", "day")

Expand All @@ -116,8 +116,11 @@ get_custom_effort <- function(package = NULL,
check_value(group_by, group_bys, "group_by", null_allowed = TRUE)

# check camera trap data package
package <- check_package(package, datapkg, "get_custom_effort")

check_package(package, datapkg, "get_custom_effort")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# get deployments
deployments <- package$data$deployments

Expand Down
7 changes: 5 additions & 2 deletions R/get_effort.R
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ get_effort <- function(package = NULL,
check_value(unit, units, "unit", null_allowed = FALSE)

# check camera trap data package
package <- check_package(package, datapkg, "get_effort")

check_package(package, datapkg, "get_effort")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# apply filtering
package$data$deployments <- apply_filter_predicate(
df = package$data$deployments,
Expand Down
7 changes: 5 additions & 2 deletions R/get_n_individuals.R
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,11 @@ get_n_individuals <- function(package = NULL,
life_stage = NULL,
datapkg = lifecycle::deprecated()) {
# check input data package
package <- check_package(package, datapkg, "get_n_individuals")

check_package(package, datapkg, "get_n_individuals")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# avoid to call variables like column names to make life easier using filter()
sex_value <- sex

Expand Down
7 changes: 5 additions & 2 deletions R/get_n_obs.R
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ get_n_obs <- function(package = NULL,
life_stage = NULL,
datapkg = lifecycle::deprecated()) {
# check input data package
package <- check_package(package, datapkg, "get_n_obs")

check_package(package, datapkg, "get_n_obs")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# avoid to call variables like column names to make life easier using filter()
sex_value <- sex

Expand Down
6 changes: 4 additions & 2 deletions R/get_n_species.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ get_n_species <- function(package = NULL,
...,
datapkg = lifecycle::deprecated()) {
# check input data package
package <- check_package(package, datapkg, "get_n_species")

check_package(package, datapkg, "get_n_species")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}
# extract observations and deployments
observations <- package$data$observations
deployments <- package$data$deployments
Expand Down
13 changes: 10 additions & 3 deletions R/get_rai.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,11 @@ get_rai <- function(package = NULL,
life_stage = NULL,
datapkg = lifecycle::deprecated()) {
# check camera trap data package
package <- check_package(package, datapkg, "get_rai")

check_package(package, datapkg, "get_rai")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

get_rai_primitive(package, ...,
use = "n_obs",
species = species,
Expand Down Expand Up @@ -135,7 +138,11 @@ get_rai_individuals <- function(package = NULL,
life_stage = NULL,
datapkg = lifecycle::deprecated()) {
# check camera trap data package
package <- check_package(package, datapkg, "get_rai_individuals")
check_package(package, datapkg, "get_rai_individuals")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

get_rai_primitive(package, ...,
use = "n_individuals",
species = species,
Expand Down
7 changes: 5 additions & 2 deletions R/get_record_table.R
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,11 @@ get_record_table <- function(package = NULL,
removeDuplicateRecords = TRUE,
datapkg = lifecycle::deprecated()) {
# check data package
package <- check_package(package, datapkg, "get_record_table", media = TRUE)

check_package(package, datapkg, "get_record_table", media = TRUE)
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# check stationCol is a valid column name
assertthat::assert_that(
stationCol %in% names(package$data$deployments),
Expand Down
6 changes: 5 additions & 1 deletion R/get_scientific_name.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@
get_scientific_name <- function(package = NULL,
vernacular_name,
datapkg = lifecycle::deprecated()) {
package <- check_package(package, datapkg, "get_scientific_name")
check_package(package, datapkg, "get_scientific_name")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

all_sn_vn <- get_species(package)

# get vernacular names for check
Expand Down
7 changes: 5 additions & 2 deletions R/get_species.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
#' get_species(mica)
get_species <- function(package = NULL, datapkg = lifecycle::deprecated()) {
# Check camera trap data package
package <- check_package(package, datapkg, "get_species", media = FALSE)

check_package(package, datapkg, "get_species", media = FALSE)
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# Get taxonomic information from package metadata
if (!"taxonomic" %in% names(package)) {
return(NULL)
Expand Down
7 changes: 5 additions & 2 deletions R/map_dep.R
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,11 @@ map_dep <- function(package = NULL,
datapkg = lifecycle::deprecated()) {

# check camera trap data package
package <- check_package(package, datapkg, "map_dep")

check_package(package, datapkg, "map_dep")
if (is.null(package) & !is.name(datapkg)) {
package <- datapkg
}

# define possible feature values
features <- c(
"n_species",
Expand Down
4 changes: 2 additions & 2 deletions R/read_camtrap_dp.R
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ read_camtrap_dp <- function(file = NULL,
}

package$data <- data
package <- check_package(package, media = media)
check_package(package, media = media)

package <- add_taxonomic_info(package)

Expand All @@ -165,7 +165,7 @@ read_camtrap_dp <- function(file = NULL,
package$data$media <- order_cols_media(package$data$media)
}

package <- check_package(package, media = media)
check_package(package, media = media)

# Inherit parsing issues from reading
attr(package$data$observations, which = "problems") <- issues_observations
Expand Down
72 changes: 1 addition & 71 deletions R/zzz.R
Original file line number Diff line number Diff line change
@@ -1,73 +1,3 @@
#' Check validity camera trap data package
#'
#' Checks the validity of a camera trap data package.
#' It checks whether the data package is a list containing an element called
#' `data` with the following resources as tibble data frames:
#' - `observations`
#' - `media`
#' - `deployments`
#'
#' @param package Camera trap data package
#' @param datapkg Deprecated. Use `package` instead.
#' @param media Has the `media` resource been loaded while reading the data
#' package? Default: `FALSE`.
#' @return A camera trap data package.
#' @noRd
check_package <- function(package = NULL,
datapkg = NULL,
function_name,
media = FALSE) {
if (lifecycle::is_present(datapkg) & !is.null(datapkg)) {
lifecycle::deprecate_warn(
when = "0.16.0",
what = paste0(function_name, "(datapkg = )"),
with = paste0(function_name, "(package = )")
)
if (is.null(package)) {
package <- datapkg
}
}
# check media arg
assertthat::assert_that(
media %in% c(TRUE, FALSE),
msg = "`media` must be a logical: TRUE or FALSE"
)
# camera trap data package is a list
assertthat::assert_that(is.list(package))
assertthat::assert_that(!is.data.frame(package))
# check existence of an element called data
assertthat::assert_that("data" %in% names(package))
# check validity data element of package: does it contain deployments and
# observations?
elements <- c("deployments", "observations")
if (media) {
elements <- c(elements, "media")
}
tables_absent <- elements[
!elements %in% names(package$data)
]
assertthat::assert_that(length(tables_absent) == 0,
msg = glue::glue(
"Can't find {tables_absent} elements in data package: {tables_absent*}",
.transformer = collapse_transformer(sep = ", ", last = " and ")
)
)
if (media) {
assertthat::assert_that(
!is.null(package$data$media),
msg = glue::glue("Can't find media in .$data.")
)
}
# check observations and deployments are data.frames
assertthat::assert_that(is.data.frame(package$data$observations))
assertthat::assert_that(is.data.frame(package$data$deployments))
# check media is a data.frame (if imported, i.e. if not NULL)
if (!is.null(package$data$media)) {
assertthat::assert_that(is.data.frame(package$data$media))
}
package
}

#' Check input value against list of provided values
#'
#' Will return error message if an input value cannot be found in list of
Expand Down Expand Up @@ -247,7 +177,7 @@ get_dep_no_obs <- function(package = NULL,
datapkg = lifecycle::deprecated()) {

# check input camera trap data package
package <- check_package(package, datapkg, "get_dep_no_obs")
check_package(package, datapkg, "get_dep_no_obs")

# extract observations and deployments
observations <- package$data$observations
Expand Down
2 changes: 1 addition & 1 deletion man/get_cam_op.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion man/get_custom_effort.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading