From 54be6518e5c85baf0ffd2af5ca7c8450b73ddab5 Mon Sep 17 00:00:00 2001
From: Kyle Baron <kyleb@metrumrg.com>
Date: Mon, 9 May 2022 10:22:16 -0500
Subject: [PATCH 1/2] only change to character if type is not set

---
 R/load_spec.R | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/R/load_spec.R b/R/load_spec.R
index c2dcc52f..6bb69a64 100755
--- a/R/load_spec.R
+++ b/R/load_spec.R
@@ -350,7 +350,9 @@ unpack_col <- function(x) {
       x$decode <- names(x$values)
     }
     x$values <- sapply(x$values, sub_null_natom, USE.NAMES=FALSE)
-    if(is.character(x$values)) x$type <- "character"
+    if(is.character(x$values) && .no("type", x)) {
+      x$type <- "character"
+    }
   }
   if(.has("source", x)) {
     x$source <- paste0(x$source, collapse = " ")  

From b9ef08b10d825d28d2b8b3c9543b9f301361259c Mon Sep 17 00:00:00 2001
From: Kyle Baron <kyleb@metrumrg.com>
Date: Mon, 9 May 2022 12:56:50 -0500
Subject: [PATCH 2/2] don't clobber type when values is character

---
 Makefile                           |  3 +++
 R/load_spec.R                      |  8 ++++----
 inst/validation/latest.R           | 15 +++++++++++++++
 inst/validation/yspec-stories.yaml | 10 ++++++++++
 tests/testthat/test-load_spec.R    | 16 ++++++++++++++++
 5 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 inst/validation/latest.R

diff --git a/Makefile b/Makefile
index bf2bfc6b..4c56e35a 100755
--- a/Makefile
+++ b/Makefile
@@ -5,6 +5,9 @@ TARBALL=${PACKAGE}_${VERSION}.tar.gz
 PKGDIR=.
 CHKDIR=.
 
+testn: 
+	Rscript inst/validation/latest.R
+
 # development cycle - bumps the version and tags based on version
 bump-dev:
 	Rscript -e 'usethis::use_version("dev")'
diff --git a/R/load_spec.R b/R/load_spec.R
index 6bb69a64..38706dff 100755
--- a/R/load_spec.R
+++ b/R/load_spec.R
@@ -333,9 +333,6 @@ unpack_col <- function(x) {
   if(.no("short",x)) {
     x[["short"]] <- x[["col"]]
   }
-  if(.no("type", x)) {
-    x[["type"]] <- "numeric"
-  }
   x <- unpack_about(x)
   x$continuous <- .has("range",x)
   if(x$continuous) {
@@ -349,11 +346,14 @@ unpack_col <- function(x) {
     if(!.has("decode",x)) {
       x$decode <- names(x$values)
     }
-    x$values <- sapply(x$values, sub_null_natom, USE.NAMES=FALSE)
+    x$values <- sapply(x$values, sub_null_natom, USE.NAMES = FALSE)
     if(is.character(x$values) && .no("type", x)) {
       x$type <- "character"
     }
   }
+  if(.no("type", x)) {
+    x[["type"]] <- "numeric"
+  }
   if(.has("source", x)) {
     x$source <- paste0(x$source, collapse = " ")  
   }
diff --git a/inst/validation/latest.R b/inst/validation/latest.R
new file mode 100644
index 00000000..2aa15075
--- /dev/null
+++ b/inst/validation/latest.R
@@ -0,0 +1,15 @@
+library(yaml)
+
+x <- yaml.load_file("inst/validation/yspec-stories.yaml")
+
+stories <- names(x)
+stories <- sub("YSP-S", "", stories)
+stories <- as.integer(stories)
+
+tests <- unlist(lapply(x,  function(x) x$tests), use.names=FALSE)
+dupt <- any(duplicated(tests))
+tests <- sub("YSP-TEST-", "", tests)
+tests <- as.integer(tests)
+
+message("last story: ", max(stories))
+message("last test: ", max(tests))
diff --git a/inst/validation/yspec-stories.yaml b/inst/validation/yspec-stories.yaml
index 2cbbd1c9..ed98e6f8 100644
--- a/inst/validation/yspec-stories.yaml
+++ b/inst/validation/yspec-stories.yaml
@@ -1,3 +1,13 @@
+YSP-S034:
+  name: ys_load - only assume character type if type is NULL
+  description: > 
+    As a user, I want yspec to infer that type is character for discrete 
+    data when type is missing and the values field has type character.
+  ProductRisk: low-risk
+  tests: YSP-TEST-0145
+
+
+# --------
 YSP-S001:
   name: Make ys_get_short_unit return list
   description: ys_get_short_unit returns list not character vector
diff --git a/tests/testthat/test-load_spec.R b/tests/testthat/test-load_spec.R
index d9aa4c1d..abfa420b 100644
--- a/tests/testthat/test-load_spec.R
+++ b/tests/testthat/test-load_spec.R
@@ -119,3 +119,19 @@ test_that("Error when values is mis-coded as list of lists [YSP-TEST-0072]", {
   )
 })
 
+test_that("ys_load - only assume character type if type is NULL [YSP-TEST-0145]", {
+  spec <- yspec:::test_spec_list(
+    list(A = list(values = letters[1:3]))
+  )
+  expect_equal(spec$A$type, "character")
+  
+  spec <- yspec:::test_spec_list(
+    list(A = list(values = c(1,2,3), type = "integer"))
+  )
+  expect_equal(spec$A$type, "integer")
+  
+  spec <- yspec:::test_spec_list(
+    list(A = list(values = letters[1:3], type = "integer"))
+  )
+  expect_equal(spec$A$type, "integer")
+})