check for non-numeric cols when converting to matrix (#594)

* check for non-numeric cols when converting to matrix

* Apply suggestions from code review

Co-authored-by: Davis Vaughan <[email protected]>

* changes based on review

* try to fix xgboost install on win

* another CXX14 attempt

* remove append arg

* yet another CXX14 attempt

* proper separators

* Makevars.win and no append

* initialize file

* create directory

* typo fix

* create .R path

* try devel xgboost instead

* remove remotes

Co-authored-by: Davis Vaughan <[email protected]>

Author: topepo

.github/workflows/R-CMD-check.yaml

@@ -37,6 +37,9 @@ jobs:
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
       R_KEEP_PKG_SOURCE: yes
+      CXX14: g++
+      CXX14STD: -std=c++1y
+      CXX14FLAGS: -Wall -g -02
 
     steps:
       - uses: actions/checkout@v2

NEWS.md

@@ -12,6 +12,10 @@
 
 * The list column produced when creating survival probability predictions is now always called `.pred` (with `.pred_survival` being used inside of the list column). 
 
+## Other Changes
+
+ * When the xy interface is used and the underlying model expects to use a matrix, a better warning is issued when predictors contain non-numeric columns (including dates). 
+
 # parsnip 0.1.7
 
 ## Model Specification Changes

R/convert_data.R

@@ -356,6 +356,14 @@ check_dup_names <- function(x, y) {
 maybe_matrix <- function(x) {
   inher(x, c("data.frame", "matrix", "dgCMatrix"), cl = match.call())
   if (is.data.frame(x)) {
+    non_num_cols <- vapply(x, function(x) !is.numeric(x), logical(1))
+    if (any(non_num_cols)) {
+      non_num_cols <- names(non_num_cols)[non_num_cols]
+      non_num_cols <- glue::glue_collapse(glue::single_quote(non_num_cols), sep = ", ")
+      msg <- glue::glue("Some columns are non-numeric. The data cannot be ",
+                        "converted to numeric matrix: {non_num_cols}.")
+      rlang::abort(msg)
+    }
     x <- as.matrix(x)
   }
   # leave alone if matrix or sparse matrix

tests/testthat/test_convert_data.R

@@ -617,4 +617,16 @@ test_that("convert to matrix", {
     inherits(parsnip::maybe_matrix(Matrix::Matrix(as.matrix(mtcars), sparse = TRUE)),
              "dgCMatrix")
   )
+
+  data(ames, package = "modeldata")
+  expect_error(
+    parsnip::maybe_matrix(ames[, c("Year_Built", "Neighborhood")]),
+    "Some columns are non-numeric. The data cannot be converted to numeric matrix: 'Neighborhood'."
+  )
+  # Also for date columns
+  data(Chicago, package = "modeldata")
+  expect_error(
+    parsnip::maybe_matrix(Chicago[, c("ridership", "date")]),
+    "Some columns are non-numeric. The data cannot be converted to numeric matrix: 'date'."
+  )
 })