Skip to content

Commit

Permalink
Replace rjson dependency by jsonlite
Browse files Browse the repository at this point in the history
There are a couple of reasons for this change.  First, rjson depends
on a recent version of R (>= 4.4.0), which is the current released
minor version, while jsonlite should be able to work with older
versions.  Second, as of the time of this writing, the CRAN task view
for Web Technologies and Services recommends using the jsonlite
package.

https://cran.r-project.org/web/views/WebTechnologies.html

Also, fromJSON() in jsonlite coerces the records in the response to a
data.frame by default, which is in principle quite convenient (and
gtools::smartbind() does not seem to be needed anymore), although it
is possible that in some cases this gives unexpected results.  Some
new bugs could appear because of this.
  • Loading branch information
castroinsua committed Sep 4, 2024
1 parent 1e7cd8a commit 33dc5cf
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 54 deletions.
3 changes: 1 addition & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ URL: https://docs.ropensci.org/paleobioDB/,
BugReports: https://github.com/ropensci/paleobioDB/issues
Imports:
curl,
gtools,
jsonlite,
maps,
rjson,
terra
Suggests:
roxygen2,
Expand Down
12 changes: 12 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
paleobioDB 1.0.0.9000
=====================

### DOCUMENTATION FIXES

* In `pbdb_scales()`, at least one parameter needs to be specified
now, so the examples in the documentation were updated accordingly.

### OTHER CHANGES

* The `rjson` dependency was replaced by `jsonlite`.

paleobioDB 1.0.0
================

Expand Down
47 changes: 6 additions & 41 deletions R/network.R
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
#' @returns data frame
#' @noRd
.parse_raw_data <- function(raw_data) {
data_list <- rjson::fromJSON(raw_data)
data_list <- jsonlite::fromJSON(raw_data)

if ("warnings" %in% names(data_list)) {
# Enumerate warnings that were returned by the PBDB API
Expand All @@ -93,31 +93,16 @@
)
}

df <- .make_data_frame(data_list$records)
df
}


#' .make_data_frame
#'
#' Makes a data frame from a list of lists
#'
#' @param reg_list data rows as a list of lists
#' @returns data frame
#' @noRd
.make_data_frame <- function(reg_list) {
if (length(reg_list) == 0) {
if (length(data_list$records) == 0) {
warning("The PBDB API returned no records for this query.", call. = FALSE)
return(data.frame())
}

dfr_rows <- lapply(reg_list, function(reg) {
as.data.frame(lapply(reg, .collapse_array_columns_map))
})
if (!is.data.frame(data_list$records)) {
stop("The records in the response were not coerced to a data.frame")
}

dfr <- do.call(gtools::smartbind, dfr_rows)
dfr <- .convert_data_frame_columns(dfr)
dfr
.convert_data_frame_columns(data_list$records)
}

#' .build_query_string
Expand All @@ -144,26 +129,6 @@
qs
}

#' .collapse_array_columns_map
#'
#' Maps multivalue elements to semicolon separated strings
#'
#' @param element a vector representing some data field
#' @returns a string with the elements of the provided vector separated
#' by semicolons if it has more than one element or the vector as it
#' was passed to the function if it has length one
#' @noRd
.collapse_array_columns_map <- function(element) {
if (length(element) > 1) {
mapped <- paste(element, collapse = ";")
} else {
mapped <- element
}

mapped
}


#' .convert_data_frame_columns
#'
#' Converts some columns (lng, lat) from character to numeric. This
Expand Down
Binary file added tests/testthat/fixtures/no_records_resp.rds
Binary file not shown.
17 changes: 6 additions & 11 deletions tests/testthat/test-network.R
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,23 @@ test_that(".parse_raw_data() replicates warnings from the API", {
)
})

test_that(".make_data_frame() warns the user if no records are returned", {
test_that(".parse_raw_data() warns the user if no records are returned", {
resp <- readRDS(test_path("fixtures", "no_records_resp.rds"))
raw_data <- .extract_response_body(resp)
expect_warning(
.make_data_frame(list()),
.parse_raw_data(raw_data),
regexp = "The PBDB API returned no records for this query."
)
})

test_that(".make_data_frame() returns a data frame", {
test_that(".parse_raw_data() returns a data frame", {
resp <- readRDS(test_path("fixtures", "valid_id_resp.rds"))
raw_data <- .extract_response_body(resp)
data_list <- rjson::fromJSON(raw_data)
df <- .make_data_frame(data_list$records)
df <- .parse_raw_data(raw_data)
expect_s3_class(df, "data.frame")
expect_identical(
names(df),
c("oid", "cid", "tna", "rnk", "tid", "oei",
"eag", "lag", "rid", "lng", "lat")
)
})

test_that(".collapse_array_columns_map() returns a length one character", {
s <- .collapse_array_columns_map(1:3)
expect_length(s, 1)
expect_equal(s, "1;2;3")
})

0 comments on commit 33dc5cf

Please sign in to comment.