Merge pull request #329 from USEPA/314-tcplLoadChem-bug

314 update tcplLoadChem/tests for bug fix involving missing spid col

Author: cthunes

Diff for: R/tcplLoadChem.R

@@ -50,7 +50,7 @@ tcplLoadChem <- function(field = NULL, val = NULL, exact = TRUE,
                          include.spid = TRUE) {
 
   if (getOption("TCPL_DRVR") == "API") {
-    if (tolower(field) != "spid") stop("When drvr option is set to 'API', only 'spid' is a valid 'field' value.")
+    if (is.null(field) || tolower(field) != "spid") stop("When drvr option is set to 'API', only 'spid' is a valid 'field' value.")
     if (!exact) exact <- TRUE
     dat <- tcplQueryAPI(resource = "data", fld = "spid", val = val, return_flds = c("spid", "chid", "casn", "chnm", "dsstox_substance_id"))
     if (!length(colnames(dat))) {
@@ -65,9 +65,9 @@ tcplLoadChem <- function(field = NULL, val = NULL, exact = TRUE,
     if (!is.null(field)) {
       vfield <- c("chid", "spid", "chnm", "casn", "code", "chem.only","dsstox_substance_id")
       if (!field %in% vfield) stop("Invalid 'field' value.")
+      if (field == "chem.only") include.spid <- FALSE
     }
 
-    
     qstring <- .ChemQ(field = field, val = val, exact = exact)
 
     dat <- tcplQuery(query = qstring, tbl=tbl)
@@ -86,9 +86,7 @@ tcplLoadChem <- function(field = NULL, val = NULL, exact = TRUE,
 
   if (include.spid) return(unique(dat, by = c("spid", "chid")))
 
-  dat <- unique(dat[ , list(chid, chnm, casn, code, dsstox_substance_id)])
-
-  unique(dat, by = c("spid", "chid"))[]
+  unique(dat[ , list(chid, chnm, casn, code, dsstox_substance_id)])
 
 }
 

Diff for: tests/testthat/test-tcplLoadChem.R

@@ -18,6 +18,12 @@ httptest::with_mock_dir("ctx", {
     expect_true(all(c("spid", "chid", "casn", "chnm", "dsstox_substance_id", "code") %in% colnames(dat)))
     expect_equal(dat$spid, test_api$spid)
   })
+  test_that("include.spid = FALSE works", {
+    expect_no_error(dat <- tcplLoadChem(field = "spid", val = test_api$spid, include.spid = FALSE))
+    expect_true(is.data.table(dat))
+    expect_equal(nrow(dat), 1)
+    expect_true(all(c("chid", "casn", "chnm", "dsstox_substance_id", "code") %in% colnames(dat)))
+  })
   test_that("exact is silently changed to TRUE if driver is API", {
     expect_no_error(dat <- tcplLoadChem(field = "spid", val = test_api$spid, exact = FALSE))
     expect_true(is.data.table(dat))
@@ -27,6 +33,7 @@ httptest::with_mock_dir("ctx", {
   })
   #error cases
   test_that("field can only be spid if driver is API", {
+    expect_error(tcplLoadChem(), "When drvr option is set to 'API', only 'spid' is a valid 'field' value.")
     expect_error(tcplLoadChem(field = "chnm", val = "fakechnm"), "When drvr option is set to 'API', only 'spid' is a valid 'field' value.")
   })
   test_that("data not found results in null data.table", {