From 33dc5cfe976cd5a076f61331c87de4601d079f3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Castro=20Insua?= Date: Wed, 4 Sep 2024 09:59:19 +0200 Subject: [PATCH] Replace rjson dependency by jsonlite 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. --- DESCRIPTION | 3 +- NEWS.md | 12 +++++ R/network.R | 47 +++----------------- tests/testthat/fixtures/no_records_resp.rds | Bin 0 -> 653 bytes tests/testthat/test-network.R | 17 +++---- 5 files changed, 25 insertions(+), 54 deletions(-) create mode 100644 tests/testthat/fixtures/no_records_resp.rds diff --git a/DESCRIPTION b/DESCRIPTION index 8e49c1f..d38c779 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -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, diff --git a/NEWS.md b/NEWS.md index 556b243..d06ded1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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 ================ diff --git a/R/network.R b/R/network.R index 7e29673..b3c064c 100644 --- a/R/network.R +++ b/R/network.R @@ -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 @@ -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 @@ -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 diff --git a/tests/testthat/fixtures/no_records_resp.rds b/tests/testthat/fixtures/no_records_resp.rds new file mode 100644 index 0000000000000000000000000000000000000000..993d061052ad1445bd3c56d40b5f0f62c2e4211b GIT binary patch literal 653 zcmV;80&@KyiwFP!000001BFseZ__Xs&bp34AZXHr#DNd1LKB*nChfWn1*&SSd`vJ} zsYM$?Fg)jNYv$UK?e0T>1P2aW5hs2Imz_C*v=e`TBifxiV7KWu?Ls6c_LIkcp4ZR2 z<~oj(akAMFCo^iNljb#U#)KIoPS!bNzArjjCn^ep1TzpDRKy#3A=^QWH7?|ffgnT$ zj4GXfsRXZVUfAr?z0T!@OZQ8eSSu<*)jN-# zSonB}M}N6coVHq?>zRHS?Pu}Uu}?O0EQ_&WSQCS$>4W3-T4Q;Q=yZ9iztm>-$8gSD z#7UpZ-p3!;U!!xC{okMVzI=FG`T2SB)}>!xD+ibNSKocRP&v5r{Myd@m6NKrm4>RP zQt-67DJtPm*0m%i#AYuU`XYlQ(&gDig0@%J0+~Xd(pc&f;FuN~vw?lvS`|=&9>IbKkfFO7e-R|)H)F{q)=9y nB!-?lfMbwqSY%hkv_&C4^hWJX9Mxx&v{wHC?FAwV#sdHVo-{gP literal 0 HcmV?d00001 diff --git a/tests/testthat/test-network.R b/tests/testthat/test-network.R index 2c97abc..b0d3499 100644 --- a/tests/testthat/test-network.R +++ b/tests/testthat/test-network.R @@ -38,18 +38,19 @@ 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), @@ -57,9 +58,3 @@ test_that(".make_data_frame() returns a data frame", { "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") -})