Merge pull request #773 from mitchelloharawild/master

Simplified Arima xreg handling
robjhyndman · Feb 5, 2019 · b9648ec · b9648ec
2 parents 6c86c63 + 532fdd5
commit b9648ec
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 23 deletions.
diff --git a/R/arima.R b/R/arima.R
@@ -289,15 +289,24 @@ forecast.Arima <- function(object, h=ifelse(object$arma[5] > 1, 2 * object$arma[
 
   use.drift <- is.element("drift", names(object$coef))
   x <- object$x <- getResponse(object)
-  usexreg <- (!is.null(xreg) | use.drift | is.element("xreg", names(object))) # | use.constant)
+  usexreg <- (use.drift | is.element("xreg", names(object))) # | use.constant)
 
-  if (!is.null(xreg)) {
-    if("data.frame" %in% class(xreg))
+  if (!is.null(xreg) && usexreg) {
+    if(!is.numeric(xreg))
       stop("xreg should be a numeric matrix or a numeric vector")
+    xreg <- as.matrix(xreg)
+    if (is.null(colnames(xreg))) {
+      colnames(xreg) <- if (ncol(xreg) == 1) "xreg" else paste("xreg", 1:ncol(xreg), sep = "")
+    }
+
     origxreg <- xreg <- as.matrix(xreg)
     h <- nrow(xreg)
   }
   else {
+    if(!is.null(xreg)){
+      warning("xreg not required by this model, ignoring the provided regressors")
+      xreg <- NULL
+    }
     origxreg <- NULL
   }
 
@@ -680,15 +689,11 @@ Arima <- function(y, order=c(0, 0, 0), seasonal=c(0, 0, 0), xreg=NULL, include.m
   }
 
   if (!is.null(xreg)) {
-    if("data.frame" %in% class(xreg))
+    if(!is.numeric(xreg))
       stop("xreg should be a numeric matrix or a numeric vector")
-    nmxreg <- deparse(substitute(xreg))
     xreg <- as.matrix(xreg)
-    if (ncol(xreg) == 1 & length(nmxreg) > 1) {
-      nmxreg <- "xreg"
-    }
     if (is.null(colnames(xreg))) {
-      colnames(xreg) <- if (ncol(xreg) == 1) nmxreg else paste(nmxreg, 1:ncol(xreg), sep = "")
+      colnames(xreg) <- if (ncol(xreg) == 1) "xreg" else paste("xreg", 1:ncol(xreg), sep = "")
     }
   }
 

diff --git a/R/newarima2.R b/R/newarima2.R
@@ -184,20 +184,11 @@ auto.arima <- function(y, d=NA, D=NA, max.p=5, max.q=5,
 
   # Check xreg and do regression if necessary
   if (!is.null(xreg)) {
-    # Make sure it is a matrix with column names
-    if("data.frame" %in% class(xreg))
-      stop("xreg should be a numeric matrix or vector")
-    nmxreg <- deparse(substitute(xreg))
+    if(!is.numeric(xreg))
+      stop("xreg should be a numeric matrix or a numeric vector")
     xregg <- as.matrix(xreg)
-    if (ncol(xregg) == 1 && length(nmxreg) > 1) {
-      nmxreg <- "xreg"
-    }
     if (is.null(colnames(xregg))) {
-      colnames(xregg) <- if (ncol(xregg) == 1) {
-        nmxreg
-      } else {
-        paste(nmxreg, 1:ncol(xregg), sep = "")
-      }
+      colnames(xregg) <- if (ncol(xregg) == 1) "xreg" else paste("xreg", 1:ncol(xregg), sep = "")
     }
 
     xx <- x

diff --git a/tests/testthat/test-arima.R b/tests/testthat/test-arima.R
@@ -65,7 +65,7 @@ if (require(testthat)) {
 
   test_that("tests for forecast.Arima", {
     fit1 <- Arima(wineind, order = c(1, 1, 2), seasonal = c(0, 1, 1), method = "CSS")
-    expect_error(forecast.Arima(fit1, xreg = 1:10))
+    expect_warning(forecast.Arima(fit1, xreg = 1:10), "xreg not required")
     expect_warning(forecast.Arima(fit1, include.drift = TRUE))
     expect_true(all.equal(forecast.Arima(fit1, bootstrap = TRUE, npaths = 100)$ mean, forecast.Arima(fit1)$mean))
 
@@ -81,7 +81,7 @@ if (require(testthat)) {
     fit4 <- Arima(wineind, order = c(1, 1, 2), seasonal = c(0, 1, 1), xreg = rnorm(length(wineind)))
     expect_error(forecast.Arima(fit4))
     expect_error(forecast.Arima(fit4, xreg = matrix(rnorm(40), ncol = 2)))
-    expect_warning(forecast.Arima(fit4, xreg = rnorm(20))$mean, "different column names") %>% 
+    forecast.Arima(fit4, xreg = rnorm(20))$mean %>% 
       length %>% 
       expect_equal(20)