Skip to content

Commit

Permalink
drop {assertthat} dependency (#243)
Browse files Browse the repository at this point in the history
jameslamb authored Feb 1, 2025
1 parent 68b3ca6 commit 2028290
Showing 12 changed files with 149 additions and 60 deletions.
2 changes: 1 addition & 1 deletion .ci/setup.sh
Original file line number Diff line number Diff line change
@@ -19,5 +19,5 @@ sudo apt-get install \
tidy \
qpdf

Rscript -e "install.packages(c('assertthat', 'covr', 'data.table', 'futile.logger', 'httr', 'jsonlite', 'knitr', 'lintr', 'markdown', 'purrr', 'stringr', 'testthat'), repos = 'https://cran.r-project.org', Ncpus = parallel::detectCores())"
Rscript -e "install.packages(c('covr', 'data.table', 'futile.logger', 'httr', 'jsonlite', 'knitr', 'lintr', 'markdown', 'purrr', 'stringr', 'testthat'), repos = 'https://cran.r-project.org', Ncpus = parallel::detectCores())"
cp test-data/* r-pkg/inst/testdata/
1 change: 0 additions & 1 deletion r-pkg/DESCRIPTION
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ Description:
Depends:
R (>= 3.3.0)
Imports:
assertthat (>= 0.2.0),
data.table,
futile.logger,
httr,
6 changes: 0 additions & 6 deletions r-pkg/NAMESPACE
Original file line number Diff line number Diff line change
@@ -6,12 +6,6 @@ export(es_search)
export(get_fields)
export(parse_date_time)
export(unpack_nested_data)
importFrom(assertthat,is.count)
importFrom(assertthat,is.flag)
importFrom(assertthat,is.number)
importFrom(assertthat,is.string)
importFrom(assertthat,is.writeable)
importFrom(assertthat,see_if)
importFrom(data.table,":=")
importFrom(data.table,as.data.table)
importFrom(data.table,copy)
77 changes: 60 additions & 17 deletions r-pkg/R/assertions.R
Original file line number Diff line number Diff line change
@@ -1,21 +1,64 @@

# [title] assert_that wrapper
# [name] assert
# [description] When making an assertion you might call:
#
# \code{assertthat::assert_that(assertthat::is.date(x))}
#
# or something like that. This is an alias to \code{\link[assertthat]{assert_that}} to be used
# for two benefits: \enumerate{
# \item{This uses \code{\link{log_fatal}} instead of \code{\link{stop}} on failure}
# \item{Much less clutter in the source code}
# }
#' @importFrom assertthat see_if
.assert <- function(..., msg = NULL) {
res <- assertthat::see_if(..., env = parent.frame(), msg = msg)
if (res) {
# [title] assert something and raise an exception if it isn't true
# [name] .assert
# [description] If the condition passed to .assert() does not evaluate to TRUE,
# issues a FATAL-level log message and then raises an R exception,
# both with the content of `msg`.
.assert <- function(expr, msg) {
res <- eval(expr, env = parent.frame())
if (isTRUE(res)) {
return(invisible(TRUE))
} else {
log_fatal(attr(res, "msg"))
}
log_fatal(msg)
}

# [title] check if an object is a count
# [name] .is_count
# [description] Returns TRUE if `x` is a single positive integer
# and FALSE otherwise.
.is_count <- function(x) {
return(
length(x) == 1 &&
is.numeric(x) &&
!is.na(x) &&
x > 0 &&
trunc(x) == x
)
}

# [title] check if an object is a scalar logical
# [name] .is_flag
# [description] Returns TRUE if `x` is `TRUE` or `FALSE`
# and `FALSE` otherwise.
.is_flag <- function(x) {
return(
is.logical(x) &&
length(x) == 1L &&
!is.na(x)
)
}

# [title] check if an object is a string
# [name] .is_string
# [description] Returns TRUE if `x` is a non-empty string
# and FALSE otherwise.
.is_string <- function(x) {
return(
is.character(x) &&
length(x) == 1L &&
!is.na(x) &&
x != ""
)
}

# [title] check if an object is a writeable filepath that exists
# [name] .is_writeable
# [description] Returns TRUE if `x` is a filepath that already exists
# and is writeable, and FALSE otherwise.
.is_writeable <- function(x) {
return(
.is_string(x) &&
file.exists(x) &&
file.access(x, mode = 2L)[[1L]] == 0L
)
}
4 changes: 2 additions & 2 deletions r-pkg/R/chomp_aggs.R
Original file line number Diff line number Diff line change
@@ -42,8 +42,8 @@ chomp_aggs <- function(aggs_json = NULL) {

# Get first agg name
aggNames <- names(jsonList[["aggregations"]])
assertthat::assert_that(
assertthat::is.string(aggNames)
.assert(
.is_string(aggNames)
, msg = "aggregations are expected to have a single user-assigned name. This is a malformed aggregations response."
)

35 changes: 12 additions & 23 deletions r-pkg/R/es_search.R
Original file line number Diff line number Diff line change
@@ -40,7 +40,6 @@
#' want to change this behavior, provide a path here. `es_search` will create
#' and write to a temporary directory under whatever path you provide.
#' @inheritParams doc_shared
#' @importFrom assertthat is.count is.flag is.number is.string is.writeable
#' @importFrom parallel detectCores
#' @export
#' @examples
@@ -88,7 +87,7 @@ es_search <- function(es_host
) {

# Check if this is an aggs or straight-up search query
if (!assertthat::is.string(query_body)) {
if (!.is_string(query_body)) {
msg <- sprintf(paste0("query_body should be a single string. ",
"You gave an object of length %s")
, length(query_body))
@@ -105,32 +104,22 @@ es_search <- function(es_host
}

# assign 1 core by default, if the number of cores is NA
if (is.na(n_cores) || !assertthat::is.count(n_cores)) {
if (is.na(n_cores) || !.is_count(n_cores)) {
msg <- "detectCores() returned NA. Assigning number of cores to be 1."
log_warn(msg)
n_cores <- 1
}

# Other input checks we don't have explicit error messages for
.assert(
assertthat::is.string(es_host)
, es_host != ""
, assertthat::is.string(es_index)
, es_index != ""
, assertthat::is.string(query_body)
, query_body != ""
, assertthat::is.string(scroll)
, scroll != ""
, max_hits >= 0
, assertthat::is.count(n_cores)
, n_cores >= 1
, assertthat::is.flag(break_on_duplicates)
, !is.na(break_on_duplicates)
, assertthat::is.flag(ignore_scroll_restriction)
, !is.na(ignore_scroll_restriction)
, assertthat::is.string(intermediates_dir)
, assertthat::is.writeable(intermediates_dir)
)
# other input checks with simple error messages
.assert(.is_string(es_host), "Argument 'es_host' must be a non-empty string")
.assert(.is_string(es_index), "Argument 'es_index' must be a non-empty string")
.assert(.is_string(query_body), "Argument 'query_body' must be a non-empty string")
.assert(.is_string(scroll), "Argument 'scroll' must be a non-empty string")
.assert(.is_count(max_hits), "Argument 'max_hits' must be a single positive integer")
.assert(.is_count(n_cores), "Argument 'n_cores' must be a single positive integer")
.assert(.is_flag(break_on_duplicates), "Argument 'break_on_duplicates' must be TRUE or FALSE")
.assert(.is_flag(ignore_scroll_restriction), "Argument 'ignore_scroll_restriction' must be TRUE or FALSE")
.assert(.is_writeable(intermediates_dir), "Argument 'intermediates_dir' must be a writeable filepath")

# Aggregation Request
if (grepl("aggs", query_body, fixed = TRUE)) {
5 changes: 3 additions & 2 deletions r-pkg/R/get_fields.R
Original file line number Diff line number Diff line change
@@ -27,9 +27,10 @@ get_fields <- function(es_host
# Input checking
es_url <- .ValidateAndFormatHost(es_host)

# other input checks with simple error messages
.assert(
is.character("es_indices")
, length(es_indices) > 0
is.character(es_indices) && length(es_indices) > 0
, "Argument 'es_indices' must be a non-empty character vector"
)

# collapse character vectors into comma separated strings. If any arguments
6 changes: 1 addition & 5 deletions r-pkg/R/parse_date_time.R
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@
#'
#' This is a side-effect-free function: it returns a new data.table and the
#' input data.table is unmodified.
#' @importFrom assertthat is.string
#' @importFrom data.table copy is.data.table
#' @importFrom purrr map2 simplify
#' @importFrom stringr str_extract
@@ -66,10 +65,7 @@ parse_date_time <- function(input_df
}

# Other input checks we don't have explicit error messages for
.assert(
assertthat::is.string(assume_tz)
, assume_tz != ""
)
.assert(.is_string(assume_tz), "Argument 'assume_tz' must be a non-empty string")

# Work on a copy of the DT to avoid side effects
outDT <- data.table::copy(input_df)
2 changes: 1 addition & 1 deletion r-pkg/R/unpack_nested_data.R
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ unpack_nested_data <- function(chomped_df, col_to_unpack) {
msg <- "For unpack_nested_data, chomped_df must be a data.table"
log_fatal(msg)
}
if (!assertthat::is.string(col_to_unpack)) {
if (!.is_string(col_to_unpack)) {
msg <- "For unpack_nested_data, col_to_unpack must be a character of length 1"
log_fatal(msg)
}
67 changes: 67 additions & 0 deletions r-pkg/tests/testthat/test-assertions.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
test_that(".is_count() works", {
expect_true(.is_count(1L))
expect_true(.is_count(8L))
expect_false(.is_count(-2L))
expect_false(.is_count(0))
expect_false(.is_count(15.2))
expect_false(.is_count(NA))
expect_false(.is_count(NA_character_))
expect_false(.is_count(NA_integer_))
expect_false(.is_count(NA_real_))
expect_false(.is_count(c(1L, 2L)))
expect_false(.is_count("a-number"))
expect_false(.is_count(NULL))
expect_false(.is_count(TRUE))
})

test_that(".is_flag() works", {
expect_true(.is_flag(TRUE))
expect_true(.is_flag(FALSE))
expect_false(.is_flag(-1))
expect_false(.is_flag(-1L))
expect_false(.is_flag(0))
expect_false(.is_flag(0L))
expect_false(.is_flag(1))
expect_false(.is_flag(1L))
expect_false(.is_flag(15.2))
expect_false(.is_flag(NA))
expect_false(.is_flag(NA_character_))
expect_false(.is_flag(NA_integer_))
expect_false(.is_flag(NA_real_))
expect_false(.is_flag(c(1L, 2L)))
expect_false(.is_flag("a-number"))
expect_false(.is_flag(NULL))
})

test_that(".is_string() works", {
expect_true(.is_string("abc"))
expect_true(.is_string(" "))
expect_false(.is_string(""))
expect_false(.is_string(-2L))
expect_false(.is_string(0))
expect_false(.is_string(15.2))
expect_false(.is_string(NA))
expect_false(.is_string(NA_character_))
expect_false(.is_string(NA_integer_))
expect_false(.is_string(NA_real_))
expect_false(.is_string(c(1L, 2L)))
expect_false(.is_string(NULL))
expect_false(.is_string(TRUE))
})

test_that(".is_writeable() works", {
expect_true(.is_writeable(getwd()))
expect_false(.is_writeable(file.path(tempdir(), "some-nonsense")))
expect_false(.is_writeable(""))
expect_false(.is_writeable(-2L))
expect_false(.is_writeable(0))
expect_false(.is_writeable(15.2))
expect_false(.is_writeable(NA))
expect_false(.is_writeable(NA_character_))
expect_false(.is_writeable(NA_integer_))
expect_false(.is_writeable(NA_real_))
expect_false(.is_writeable(c(1L, 2L)))
expect_false(.is_writeable("a-number"))
expect_false(.is_writeable(NULL))
expect_false(.is_writeable(TRUE))
})
2 changes: 1 addition & 1 deletion r-pkg/tests/testthat/test-get_fields.R
Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ futile.logger::flog.threshold(0)
test_that("get_fields should give an informative error if es_indices is NULL or an empty string", {
expect_error(get_fields(es_host = "http://es.custdb.mycompany.com:9200"
, es_indices = NULL),
regexp = "not greater than 0")
regexp = "Argument 'es_indices' must be a non-empty character vector")
expect_error(get_fields(es_host = "http://es.custdb.mycompany.com:9200"
, es_indices = ""),
regexp = "get_fields must be passed a valid es_indices")
2 changes: 1 addition & 1 deletion r-pkg/tests/testthat/test-integration.R
Original file line number Diff line number Diff line change
@@ -194,7 +194,7 @@ futile.logger::flog.threshold(0)
ver <- uptasticsearch:::.get_es_version(es_host = "http://127.0.0.1:9200")

# is a string
expect_true(assertthat::is.string(ver), info = paste0("returned version: ", ver))
expect_true(.is_string(ver), info = paste0("returned version: ", ver))

# Decided to check that it's coercible to an integer instead of
# hard-coding known Elasticsearch versions so this test won't require

0 comments on commit 2028290

Please sign in to comment.