From a9f3781f171ad167e0f54d2a0b2c93d9425f6357 Mon Sep 17 00:00:00 2001
From: Nicholas Potter <potterzot@gmail.com>
Date: Mon, 23 Sep 2019 14:50:32 -0700
Subject: [PATCH 1/6] make ens and time dimensions specific to variable being
 called.

---
 R/gefs.R                   | 70 ++++++++++++++++++++++++++++++--------
 tests/testthat/test-gefs.R | 56 ++++++++++++++++++++++--------
 2 files changed, 97 insertions(+), 29 deletions(-)

diff --git a/R/gefs.R b/R/gefs.R
index 73e265af..35c2872c 100644
--- a/R/gefs.R
+++ b/R/gefs.R
@@ -58,12 +58,12 @@
 #'
 #' #Get forecast for a certain variable.
 #' forecast <- gefs("Total_precipitation_surface_6_Hour_Accumulation_ens",
-#'   lat, lon)
+#'   lat, lon, ens = 0, time = 12)
 #'
 #' #Fetch a different date (available up to 10 days prior to today)
 #' forecast_yesterday_prec <- gefs(
 #'    "Total_precipitation_surface_6_Hour_Accumulation_ens",
-#'    lat, lon, date=format(as.Date(Sys.time()) - 1, "%Y%m%d"))
+#'    lat, lon, ens = 1, time = 6, date=format(as.Date(Sys.time()) - 1, "%Y%m%d"))
 #'
 #' #specific ensemble and times, for the 1800 forecast.
 #' # here ensembles 1-3 (ensembles are numbered starting with 0)
@@ -107,11 +107,11 @@ gefs_CONNECT <- function(date = format(Sys.time(), "%Y%m%d"),
 }
 
 #' @rdname gefs
-gefs_GET <- function(var, lat, lon,
+gefs_GET <- function(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
                      date = format(Sys.time(), "%Y%m%d"),
                      forecast_time = c("0000", "0600", "1200", "1800"),
-                     ens_idx = 1:21,
-                     time_idx = 1:65,
+                     ens_idx = NULL, # will be removed in future version
+                     time_idx = NULL, # will be removed in future version
                      dims = NULL,
                      raw = FALSE,
                      ...) {
@@ -147,24 +147,42 @@ gefs_GET <- function(var, lat, lon,
   n_time <- varsize[ndims] #time is always the last dimension
 
   # Set the indices for each dimension
+  dim_names <- sapply(v$dim, function(d) { d$name })
   dim_idxs <- list()
   for (i in 1:length(v$dim)) {
     dn <- v$dim[[i]]$name
     if(dn == "lon") {
-      dim_idxs[[i]] <- if(!missing(lon)) which(v$dim[[i]]$vals %in% (round(lon,0) %% 360)) else 1:v$dim[[i]]$len
+      dim_idxs[[i]] <- if(!is.null(lon)) which(v$dim[[i]]$vals %in% (round(lon,0) %% 360)) else 1:v$dim[[i]]$len
     } else if (dn == "lat") {
-      dim_idxs[[i]] <- if(!missing(lat)) which(v$dim[[2]]$vals %in% round(lat, 0)) else 1:v$dim[[2]]$len
+      dim_idxs[[i]] <- if(!is.null(lat)) which(v$dim[[i]]$vals %in% round(lat, 0)) else 1:v$dim[[i]]$len
     } else if (dn == "ens") {
-      dim_idxs[[i]] <- ens_idx
-    } else if (dn %in% c("time1", "time2")) {
-      dim_idxs[[i]] <- time_idx
+      dim_idxs[[i]] <- if(!is.null(ens)) which(v$dim[[i]]$vals %in% ens) else 1:v$dim[[i]]$len
+    } else if (dn %in% c("time", "time1", "time2")) {
+      dim_idxs[[i]] <- if(!is.null(ens)) which(v$dim[[i]]$vals %in% time) else 1:v$dim[[i]]$len
     } else if (dn %in% names(additional_dims)) {
       dim_idxs[[i]] <- which(v$dim[[j]]$vals %in% additional_dims[[dn]])
     } else {
       dim_idxs[[i]] <- 1:v$dim[[i]]$len
     }
   }
-  names(dim_idxs) <- lapply(1:length(v$dim), function(i) { v$dim[[i]]$name })
+  names(dim_idxs) <- dim_names
+
+  # Assign ens_idx and time_idx if used
+  if(!is.null(ens_idx)) {
+    d_idx <- which(dim_names == "ens")
+    if(!is.null(ens_idx)) message("'ens_idx' is depreciated and will be removed in future versions, please specify values (not indices) in 'ens' instead.")
+    if(!any(ens_idx %in% 1:v$dim[[d_idx]]$len)) stop("'ens_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'ens').")
+    dim_idxs[['ens']] <- ens_idx
+  }
+  if(!is.null(time_idx)) {
+    time_name <- grep("time", dim_names, value = TRUE)
+    d_idx <- which(dim_names %in% time_name)
+    if(!is.null(time_idx)) message("'time_idx' is depreciated and will be removed in future versions, please specify values (not indices) in 'time' instead.")
+    if(!any(time_idx %in% 1:v$dim[[d_idx]]$len)) stop("'time_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'time').")
+    dim_idxs[[time_name]] <- time_idx
+  }
+
+
 
   #start indices of dimensions to read from data
   start <- sapply(dim_idxs, function(d) { min(d) })
@@ -189,7 +207,7 @@ gefs_GET <- function(var, lat, lon,
   forecast_time <- strsplit(fname, ".grib2")[[8]]
   list(forecast_date = date,
        forecast_time = forecast_time,
-       dimensions    = dims,
+       dimensions    = names(dim_idxs),
        data          = d)
 }
 
@@ -228,9 +246,31 @@ gefs_dimensions <- function(con = NULL, ...) {
 #'
 #' @param dim (character) the dimension.
 #' @rdname gefs
-gefs_dimension_values <- function(dim, con = NULL, ...) {
-  if (is.null(dim) || missing(dim)) stop("dim cannot be NULL or missing.")
+gefs_dimension_values <- function(dim, var = NULL, con = NULL, ...) {
+  if (missing(dim)) stop("dim cannot be NULL or missing.")
   if (is.null(con)) con = gefs_CONNECT(...)
-  con$dim[[dim]]$vals
+
+  if (!(dim %in% names(con$dim))) {
+    stop(paste0(dim, " is not a valid GEFS dimension. Check with 'gefs_dimensions()'."))
+  }
+
+  if (!is.null(var)) {
+    v <- con$var[[var]]
+    dim_names <- sapply(v$dim, function(d) { d$name })
+
+    # there are multiple "time" dimensions, so get the one for this variable
+    if(dim == "time") dim <- grep("time", dim_names, value = TRUE)
+
+    if(!(dim %in% dim_names)) stop(paste0(dim,
+                                          " is not in variable dimensions: ",
+                                         paste0(dim_names, collapse = ", "),
+                                         "."))
+
+    dim_idx <- which(dim_names == dim)
+    res = con$var[[var]]$dim[[dim_idx]]$vals
+  } else {
+    res <- con$dim[[dim]]$vals
+  }
+  res
 }
 
diff --git a/tests/testthat/test-gefs.R b/tests/testthat/test-gefs.R
index 9f248064..8a3614dc 100644
--- a/tests/testthat/test-gefs.R
+++ b/tests/testthat/test-gefs.R
@@ -13,12 +13,14 @@ test_that("gefs errors", {
   skip_on_travis()
 
   expect_error(gefs(lat=lat, lon=lon), "Need to specify the variable to get. A list of variables is available from gefs_variables().")
-  expect_error(gefs(var = temp, lat = c(-43, -41), lon = lons, ens_idx = 1, time_idx = 1), "Latitudes must be sequential.", fixed = TRUE)
-  expect_error(gefs(var = temp, lat = lats, lon = c(213, 211), ens_idx = 1, time_idx = 1), "Longitudes must be sequential.", fixed = TRUE)
-  expect_error(gefs(var = temp, lat = -91, lon = lons, ens_idx = 1, time_idx = 1), "Latitudes must be in c(-90,90).", fixed = TRUE)
-  expect_error(gefs(var = temp, lat = 91, lon = lons, ens_idx = 1, time_idx = 1), "Latitudes must be in c(-90,90).", fixed = TRUE)
-  expect_error(gefs(var = temp, lat = lats, lon = 361, ens_idx = 1, time_idx = 1), "Longitudes must be in c(-180,180) or c(0,360).", fixed = TRUE)
-  expect_error(gefs(var = temp, lat = lats, lon = -181, ens_idx = 1, time_idx = 1), "Longitudes must be in c(-180,180) or c(0,360).", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = c(-43, -41), lon = lons, ens = 1, time = 6), "Latitudes must be sequential.", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = lats, lon = c(213, 211), ens = 1, time = 6), "Longitudes must be sequential.", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = -91, lon = lons, ens = 1, time = 6), "Latitudes must be in c(-90,90).", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = 91, lon = lons, ens = 1, time = 6), "Latitudes must be in c(-90,90).", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = lats, lon = 361, ens = 1, time = 6), "Longitudes must be in c(-180,180) or c(0,360).", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = lats, lon = -181, ens = 1, time = 6), "Longitudes must be in c(-180,180) or c(0,360).", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = lats, lon = lons, ens_idx = 22, time_idx = 1), "'ens_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'ens').", fixed = TRUE)
+  expect_error(gefs(var = temp, lat = lats, lon = lons, ens_idx = 1, time_idx = 67), "'time_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'time').", fixed = TRUE)
 })
 
 test_that("gefs HTTP requests", {
@@ -28,13 +30,13 @@ test_that("gefs HTTP requests", {
 
   ### Get raw and processed data
   d_raw <- gefs(var = temp,
-                ens_idx = 1:2,
-                time_idx = 1:2,
+                ens = 1:2,
+                time = c(6,12),
                 forecast_time = "0000",
                 lon = lons, lat = lats, raw = TRUE)
   d <- gefs(var = temp,
-            ens_idx = 1:2,
-            time_idx = 1:2,
+            ens = 0:1,
+            time = c(6,12),
             forecast_time = "0000",
             lon = lons, lat = lats)
 
@@ -50,7 +52,7 @@ test_that("gefs HTTP requests", {
   expect_true(all(sort(unique(d$data$lon)) == sort(round(lons %% 360, 0))))
   expect_true(all(sort(unique(d$data$lat)) == sort(round(lats, 0))))
 
-  ### Tests of data transformation from multidimensional array to data frame  
+  ### Tests of data transformation from multidimensional array to data frame
   grid <- expand.grid(lon = round(lons %% 360, 0), lat = round(lats, 0),
                       ens  = 1:2, time = 1:2)
 
@@ -70,7 +72,29 @@ test_that("gefs HTTP requests", {
     d$data$time2 == 1,][[temp]]))
 
 })
-                                                    
+
+test_that("ens_idx and time_idx replace ens and time values", {
+  skip_on_cran()
+  skip_on_travis()
+  skip_on_appveyor()
+
+  d <- gefs(var = temp,
+            ens = 0,
+            time = 6,
+            ens_idx = 1:2,
+            forecast_time = "0000",
+            lon = lons, lat = lats)
+  expect_true(all(0:1 %in% unique(d$data$ens)))
+
+  d <- gefs(var = temp,
+            ens = 0,
+            time = 6,
+            time_idx = 3:4,
+            forecast_time = "0000",
+            lon = lons, lat = lats)
+  expect_true(all(c(12,18) %in% unique(d$data$time1)))
+})
+
 test_that("gefs_variables returns characters.", {
   skip_on_cran()
   skip_on_travis()
@@ -112,6 +136,12 @@ test_that("gefs_dimensions returns character list.", {
   expect_is(dims[1], "character")
 })
 
+test_that("gefs_dimension_values errors", {
+  expect_error(gefs_dimension_values(dim = "time2", var = temp), "time2 is not in variable dimensions: lon, lat, height_above_ground, ens, time1.", fixed = TRUE)
+  expect_error(gefs_dimension_values(), "dim cannot be NULL or missing.", fixed = TRUE)
+  expect_error(gefs_dimension_values(dim = "ens1"), "ens1 is not a valid GEFS dimension. Check with 'gefs_dimensions()'.", fixed = TRUE)
+})
+
 test_that("gefs_dimension_values returns numeric array.", {
   skip_on_cran()
   skip_on_travis()
@@ -120,6 +150,4 @@ test_that("gefs_dimension_values returns numeric array.", {
   vals = gefs_dimension_values("lat")
   expect_is(vals, "array")
   expect_is(vals[1], "numeric")
-
-  expect_error(gefs_dimension_values(dim = NULL), "dim cannot be NULL or missing.")
 })

From 5a9a1d5adaee916f728dc3d5e8cd2e8848c62338 Mon Sep 17 00:00:00 2001
From: Nicholas Potter <potterzot@gmail.com>
Date: Thu, 26 Sep 2019 13:41:43 -0700
Subject: [PATCH 2/6] Update examples to match new syntax and rebuild
 documentation.

---
 R/gefs.R    |  8 ++++----
 man/gefs.Rd | 21 +++++++++++----------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/R/gefs.R b/R/gefs.R
index 35c2872c..18c7bf6d 100644
--- a/R/gefs.R
+++ b/R/gefs.R
@@ -67,15 +67,15 @@
 #'
 #' #specific ensemble and times, for the 1800 forecast.
 #' # here ensembles 1-3 (ensembles are numbered starting with 0)
-#' # and time for 2 days from today at 1800
+#' # and two time periods: c(1800, 2400)
 #' date <- format(as.Date(Sys.time()) - 1, "%Y%m%d")
 #' var <- "Temperature_height_above_ground_ens"
-#' gefs(var, lat, lon, date = date, forecast_time = "1800", ens_idx=2:4,
-#'   time_idx=1:8)
+#' gefs(var, lat, lon, date = date, forecast_time = "1800", ens=1:3,
+#'   time=6*(3:4))
 #'
 #' #One ensemble, all latitudes and longitudes (this is a big file) for the
 #' # next 3 days.
-#' # gefs(var, ens=1, time=1:12)
+#' # gefs(var, ens=1, time=6*(1:12))
 #' }
 #'
 gefs <- function(var, lat, lon, ...) {
diff --git a/man/gefs.Rd b/man/gefs.Rd
index e093e5c2..fef1ed9b 100644
--- a/man/gefs.Rd
+++ b/man/gefs.Rd
@@ -16,9 +16,10 @@ gefs(var, lat, lon, ...)
 gefs_CONNECT(date = format(Sys.time(), "\%Y\%m\%d"),
   forecast_time = c("0000", "0600", "1200", "1800"))
 
-gefs_GET(var, lat, lon, date = format(Sys.time(), "\%Y\%m\%d"),
-  forecast_time = c("0000", "0600", "1200", "1800"), ens_idx = 1:21,
-  time_idx = 1:65, dims = NULL, raw = FALSE, ...)
+gefs_GET(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
+  date = format(Sys.time(), "\%Y\%m\%d"), forecast_time = c("0000",
+  "0600", "1200", "1800"), ens_idx = NULL, time_idx = NULL,
+  dims = NULL, raw = FALSE, ...)
 
 gefs_latitudes(con = NULL, ...)
 
@@ -28,7 +29,7 @@ gefs_variables(con = NULL, ...)
 
 gefs_dimensions(con = NULL, ...)
 
-gefs_dimension_values(dim, con = NULL, ...)
+gefs_dimension_values(dim, var = NULL, con = NULL, ...)
 }
 \arguments{
 \item{var}{the variable to get. Must be one of the variables listed in
@@ -97,24 +98,24 @@ lon <- -118.2188
 
 #Get forecast for a certain variable.
 forecast <- gefs("Total_precipitation_surface_6_Hour_Accumulation_ens",
-  lat, lon)
+  lat, lon, ens = 0, time = 12)
 
 #Fetch a different date (available up to 10 days prior to today)
 forecast_yesterday_prec <- gefs(
    "Total_precipitation_surface_6_Hour_Accumulation_ens",
-   lat, lon, date=format(as.Date(Sys.time()) - 1, "\%Y\%m\%d"))
+   lat, lon, ens = 1, time = 6, date=format(as.Date(Sys.time()) - 1, "\%Y\%m\%d"))
 
 #specific ensemble and times, for the 1800 forecast.
 # here ensembles 1-3 (ensembles are numbered starting with 0)
-# and time for 2 days from today at 1800
+# and two time periods: c(1800, 2400)
 date <- format(as.Date(Sys.time()) - 1, "\%Y\%m\%d")
 var <- "Temperature_height_above_ground_ens"
-gefs(var, lat, lon, date = date, forecast_time = "1800", ens_idx=2:4,
-  time_idx=1:8)
+gefs(var, lat, lon, date = date, forecast_time = "1800", ens=1:3,
+  time=6*(3:4))
 
 #One ensemble, all latitudes and longitudes (this is a big file) for the
 # next 3 days.
-# gefs(var, ens=1, time=1:12)
+# gefs(var, ens=1, time=6*(1:12))
 }
 
 }

From 5551731d6890ed07533a9c316ab5b0f67a23a96c Mon Sep 17 00:00:00 2001
From: Nicholas Potter <potterzot@gmail.com>
Date: Mon, 28 Oct 2019 20:08:14 -0700
Subject: [PATCH 3/6] fix test errors by requesting data from two days ago and
 fixing time name typo.

---
 tests/testthat/test-gefs.R | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tests/testthat/test-gefs.R b/tests/testthat/test-gefs.R
index afd1973e..ca09c765 100644
--- a/tests/testthat/test-gefs.R
+++ b/tests/testthat/test-gefs.R
@@ -30,15 +30,19 @@ test_that("gefs HTTP requests", {
   skip_on_appveyor()
   skip_on_os("windows")
 
+  two_days_ago <- format(Sys.Date() - 2, "%Y%m%d")
+
   ### Get raw and processed data
   d_raw <- gefs(var = temp,
                 ens = 1:2,
                 time = c(6,12),
+		date = two_days_ago,
                 forecast_time = "0000",
                 lon = lons, lat = lats, raw = TRUE)
   d <- gefs(var = temp,
             ens = 0:1,
             time = c(6,12),
+	    date = two_days_ago,
             forecast_time = "0000",
             lon = lons, lat = lats)
 
@@ -46,7 +50,7 @@ test_that("gefs HTTP requests", {
   expect_type(d, "list")
   expect_equal(names(d),
                c("forecast_date", "forecast_time", "dimensions", "data"))
-  expect_equal(d$forecast_date, format(Sys.time(), "%Y%m%d"))
+  expect_equal(d$forecast_date, two_days_ago)
   expect_equal(d$forecast_time, "0000")
   expect_s3_class(d$data, "data.frame")
 
@@ -85,6 +89,7 @@ test_that("ens_idx and time_idx replace ens and time values", {
             ens = 0,
             time = 6,
             ens_idx = 1:2,
+    	    date = format(Sys.Date() - 2, "%Y%m%d"),
             forecast_time = "0000",
             lon = lons, lat = lats)
   expect_true(all(0:1 %in% unique(d$data$ens)))
@@ -93,10 +98,10 @@ test_that("ens_idx and time_idx replace ens and time values", {
             ens = 0,
             time = 6,
             time_idx = 3:4,
+   	    date = format(Sys.Date() - 2, "%Y%m%d"),
             forecast_time = "0000",
             lon = lons, lat = lats)
-  #expect_true(all(c(12,18) %in% unique(d$data$time1)))
-  expect_true(all(c(12,18) %in% unique(d$data$time2)))
+  expect_true(all(c(12,18) %in% unique(d$data$time)))
 })
 
 test_that("gefs_variables returns characters.", {
@@ -147,16 +152,14 @@ test_that("gefs_dimensions returns character list.", {
 test_that("gefs_dimension_values errors", {
   skip_on_os("windows")
   
-  # FIXME: this doesn't error anymore, ask Potter
-  # expect_error(gefs_dimension_values(dim = "time2", var = temp),
-  #   "time2 is not in variable dimensions: lon, lat, height_above_ground, ens, time1.",
-  #   fixed = TRUE)
+  expect_error(gefs_dimension_values(dim = "time1", var = temp),
+    "time1 is not in variable dimensions: lon, lat, height_above_ground, ens, time2.",
+    fixed = TRUE)
   expect_error(gefs_dimension_values(),
     "dim cannot be NULL or missing.", fixed = TRUE)
   expect_error(gefs_dimension_values(dim = "ens1"),
     "ens1 is not a valid GEFS dimension. Check with 'gefs_dimensions()'.",
     fixed = TRUE)
->>>>>>> upstream/master
 })
 
 test_that("gefs_dimension_values returns numeric array.", {

From 9e5b8fff8633215d90c876fc881ec400d4691a0b Mon Sep 17 00:00:00 2001
From: Nicholas Potter <potterzot@gmail.com>
Date: Tue, 29 Oct 2019 09:52:31 -0700
Subject: [PATCH 4/6] minor fix to test to fix dimension name.

---
 tests/testthat/test-gefs.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-gefs.R b/tests/testthat/test-gefs.R
index ca09c765..a0dd6fe3 100644
--- a/tests/testthat/test-gefs.R
+++ b/tests/testthat/test-gefs.R
@@ -153,7 +153,7 @@ test_that("gefs_dimension_values errors", {
   skip_on_os("windows")
   
   expect_error(gefs_dimension_values(dim = "time1", var = temp),
-    "time1 is not in variable dimensions: lon, lat, height_above_ground, ens, time2.",
+    "time2 is not in variable dimensions: lon, lat, height_above_ground, ens, time1.",
     fixed = TRUE)
   expect_error(gefs_dimension_values(),
     "dim cannot be NULL or missing.", fixed = TRUE)

From 3df65df22fd84afeb0142e31f6c2dd96b51a95fc Mon Sep 17 00:00:00 2001
From: Nicholas Potter <potterzot@gmail.com>
Date: Tue, 29 Oct 2019 09:53:45 -0700
Subject: [PATCH 5/6] fix the fix with an eyeroll...

---
 tests/testthat/test-gefs.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-gefs.R b/tests/testthat/test-gefs.R
index a0dd6fe3..1d8b84ab 100644
--- a/tests/testthat/test-gefs.R
+++ b/tests/testthat/test-gefs.R
@@ -152,7 +152,7 @@ test_that("gefs_dimensions returns character list.", {
 test_that("gefs_dimension_values errors", {
   skip_on_os("windows")
   
-  expect_error(gefs_dimension_values(dim = "time1", var = temp),
+  expect_error(gefs_dimension_values(dim = "time2", var = temp),
     "time2 is not in variable dimensions: lon, lat, height_above_ground, ens, time1.",
     fixed = TRUE)
   expect_error(gefs_dimension_values(),

From b73b306a34ca7a583d942fe527831467dbde8a2d Mon Sep 17 00:00:00 2001
From: Nicholas Potter <potterzot@gmail.com>
Date: Thu, 31 Oct 2019 15:52:34 -0700
Subject: [PATCH 6/6] remove leftover connection files from errors and add
 dimension convenience functions.

---
 NAMESPACE                  |   2 +
 R/gefs.R                   | 103 +++++++++++++++++++++++++++----------
 man/gefs.Rd                |  38 ++++++++++----
 tests/testthat/test-gefs.R |  70 ++++++++++++++++++-------
 4 files changed, 157 insertions(+), 56 deletions(-)

diff --git a/NAMESPACE b/NAMESPACE
index 3489e83c..b5536664 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -39,8 +39,10 @@ export(ersst)
 export(gefs)
 export(gefs_dimension_values)
 export(gefs_dimensions)
+export(gefs_ensembles)
 export(gefs_latitudes)
 export(gefs_longitudes)
+export(gefs_times)
 export(gefs_variables)
 export(ghcnd)
 export(ghcnd_clear_cache)
diff --git a/R/gefs.R b/R/gefs.R
index 46b0f670..630fcff4 100644
--- a/R/gefs.R
+++ b/R/gefs.R
@@ -7,7 +7,7 @@
 #' @export
 #'
 #' @param var the variable to get. Must be one of the variables listed in
-#' `gefs_variables()`
+#' `gefs_variables()`.
 #' @param lat the latitude. Values must be sequential and are rounded to the
 #' nearest GEFS available latitude.
 #' @param lon the longitude. Values must be sequential and are rounded to the
@@ -25,7 +25,7 @@
 #' included between lat, lon, ens, and time.
 #' @param raw logical to indicate whether to return raw data matrix or
 #' reshaped data frame.
-#' @param ... additional parameters passed to `ncvar_get`
+#' @param ... additional parameters passed to the connection.
 #' @return a list containing metadata and accompanying data frame of
 #' forecast values. If lat/lon are not specified, the $data is an
 #' unprocessed matrix.
@@ -102,7 +102,6 @@ gefs_CONNECT <- function(date = format(Sys.time(), "%Y%m%d"),
   gefs_url <- paste0(gefs_url_pre, date, "_", forecast_time, gefs_url_suf)
 
   #open the connection
-  #nc_open(gefs_url) #ncdf4 version
   ncdf4::nc_open(gefs_url)
 }
 
@@ -133,7 +132,8 @@ gefs_GET <- function(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
   con <- gefs_CONNECT(date, forecast_time)
 
   # Rename extra dimensions, to be changed later
-  if (!is.null(dims)) { warning("Can't select additional dimensions yet.",
+  if (!is.null(dims)) {
+    warning("Can't select additional dimensions yet.",
                                 .call = FALSE)
   } else {
     additional_dims <- dims
@@ -171,14 +171,20 @@ gefs_GET <- function(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
   if(!is.null(ens_idx)) {
     d_idx <- which(dim_names == "ens")
     if(!is.null(ens_idx)) message("'ens_idx' is deprecated and will be removed in future versions, please specify values (not indices) in 'ens' instead.")
-    if(!any(ens_idx %in% 1:v$dim[[d_idx]]$len)) stop("'ens_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'ens').")
+    if(!any(ens_idx %in% 1:v$dim[[d_idx]]$len)) {
+      ncdf4::nc_close(con)
+      stop("'ens_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'ens').")
+    }
     dim_idxs[['ens']] <- ens_idx
   }
   if(!is.null(time_idx)) {
     time_name <- grep("time", dim_names, value = TRUE)
     d_idx <- which(dim_names %in% time_name)
     if(!is.null(time_idx)) message("'time_idx' is deprecated and will be removed in future versions, please specify values (not indices) in 'time' instead.")
-    if(!any(time_idx %in% 1:v$dim[[d_idx]]$len)) stop("'time_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'time').")
+    if(!any(time_idx %in% 1:v$dim[[d_idx]]$len)) {
+      ncdf4::nc_close(con)
+      stop("'time_idx' is out of bounds, check the dimension values with 'gefs_dimension_values(dim = 'time').")
+    }
     dim_idxs[[time_name]] <- time_idx
   }
 
@@ -190,6 +196,7 @@ gefs_GET <- function(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
 
   ##ncdf4 version
   d_raw <- ncdf4::ncvar_get(con, v, start = start, count = count_n, ...)
+  ncdf4::nc_close(con)
 
   #create the data frame
   #For now, if lat/lon are not specified, just return a matrix.
@@ -216,42 +223,79 @@ gefs_GET <- function(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
 
 #' @export
 #'
-#' @param con an ncdf4 connection.
+#' @param ... connection parameters passed to \code{gefs_dimension_values()}.
+#' @rdname gefs
+gefs_latitudes <- function(...) {
+  gefs_dimension_values(dim = "lat", ...)  
+}
+
+#' @export
+#' 
+#' @param ... connection parameters passed to \code{gefs_dimension_values()}.
 #' @rdname gefs
-gefs_latitudes <- function(con = NULL, ...) {
-  gefs_dimension_values("lat", con)
+gefs_longitudes <- function(...) {
+  gefs_dimension_values(dim = "lon", ...)
 }
 
 #' @export
+#'
+#' @param ... connection parameters passed to \code{gefs_dimension_values()}.
 #' @rdname gefs
-gefs_longitudes <- function(con = NULL, ...) {
-  gefs_dimension_values("lon", con)
+gefs_ensembles <- function(...) {
+  gefs_dimension_values(dim = "ens", ...)  
 }
 
 #' @export
+#'
+#' @param ... connection parameters passed to \code{gefs_dimension_values()}.
 #' @rdname gefs
-gefs_variables <- function(con = NULL, ...) {
-  if (is.null(con)) con = gefs_CONNECT(...)
-  names(con$var)
+gefs_times <- function(...) {
+  gefs_dimension_values(dim = "time", ...)  
 }
 
 #' @export
+#' 
+#' @param ... connection parameters passed to \code{gefs_dimension_values()}.
 #' @rdname gefs
-gefs_dimensions <- function(con = NULL, ...) {
-  if (is.null(con)) con = gefs_CONNECT(...)
-  names(con$dim)
+gefs_variables <- function(...) {
+  con = gefs_CONNECT(...)
+  vars <- names(con$var)
+  ncdf4::nc_close(con)
+  vars
 }
 
 #' @export
 #'
-#' @param dim (character) the dimension.
+#' @param var (character) the variable for which to get dimensions.
 #' @rdname gefs
-gefs_dimension_values <- function(dim, var = NULL, con = NULL, ...) {
+gefs_dimensions <- function(var = NULL, ...) {
+
+  con = gefs_CONNECT(...)
+  if(is.null(var)) {
+    dims <- names(con$dim)
+  } else {
+    v <- con$var[[var]]
+    dims <- sapply(v$dim, function(d) { d$name })
+  }  
+  ncdf4::nc_close(con)
+  dims
+  
+}
+
+#' @export
+#'
+#' @param dim (character) the dimension to fetch values for.
+#' @param var (character) the variable for which to get dimension values for.
+#' @param ... additional parameters.
+#' @rdname gefs
+gefs_dimension_values <- function(dim, var = NULL, ...) {
   if (missing(dim)) stop("dim cannot be NULL or missing.")
-  if (is.null(con)) con = gefs_CONNECT(...)
 
+  con = gefs_CONNECT(...)
+  
   if (!(dim %in% names(con$dim))) {
-    stop(paste0(dim, " is not a valid GEFS dimension. Check with 'gefs_dimensions()'."))
+    ncdf4::nc_close(con)
+    stop(paste0(dim, " is not a valid GEFS dimension. Get valid dimensions with 'gefs_dimensions()'."))
   }
 
   if (!is.null(var)) {
@@ -259,18 +303,21 @@ gefs_dimension_values <- function(dim, var = NULL, con = NULL, ...) {
     dim_names <- sapply(v$dim, function(d) { d$name })
 
     # there are multiple "time" dimensions, so get the one for this variable
-    if(dim == "time") dim <- grep("time", dim_names, value = TRUE)
-
-    if(!(dim %in% dim_names)) stop(paste0(dim,
-                                          " is not in variable dimensions: ",
-                                         paste0(dim_names, collapse = ", "),
-                                         "."))
-
+    if(grepl("time", dim)) dim <- grep("time", dim_names, value = TRUE)
+    if(!(dim %in% dim_names)) {
+      ncdf4::nc_close(con)
+      stop(paste0(dim,
+                  " is not in variable dimensions: ",
+                  paste0(dim_names, collapse = ", "),
+                  "."))
+    }
+    
     dim_idx <- which(dim_names == dim)
     res = con$var[[var]]$dim[[dim_idx]]$vals
   } else {
     res <- con$dim[[dim]]$vals
   }
+  ncdf4::nc_close(con)
   res
 }
 
diff --git a/man/gefs.Rd b/man/gefs.Rd
index 48f18d3a..b190537e 100644
--- a/man/gefs.Rd
+++ b/man/gefs.Rd
@@ -6,6 +6,8 @@
 \alias{gefs_GET}
 \alias{gefs_latitudes}
 \alias{gefs_longitudes}
+\alias{gefs_ensembles}
+\alias{gefs_times}
 \alias{gefs_variables}
 \alias{gefs_dimensions}
 \alias{gefs_dimension_values}
@@ -21,19 +23,23 @@ gefs_GET(var, lat = NULL, lon = NULL, ens = NULL, time = NULL,
   "0600", "1200", "1800"), ens_idx = NULL, time_idx = NULL,
   dims = NULL, raw = FALSE, ...)
 
-gefs_latitudes(con = NULL, ...)
+gefs_latitudes(...)
 
-gefs_longitudes(con = NULL, ...)
+gefs_longitudes(...)
 
-gefs_variables(con = NULL, ...)
+gefs_ensembles(...)
 
-gefs_dimensions(con = NULL, ...)
+gefs_times(...)
 
-gefs_dimension_values(dim, var = NULL, con = NULL, ...)
+gefs_variables(...)
+
+gefs_dimensions(var = NULL, ...)
+
+gefs_dimension_values(dim, var = NULL, ...)
 }
 \arguments{
 \item{var}{the variable to get. Must be one of the variables listed in
-\code{gefs_variables()}}
+\code{gefs_variables()}.}
 
 \item{lat}{the latitude. Values must be sequential and are rounded to the
 nearest GEFS available latitude.}
@@ -41,7 +47,7 @@ nearest GEFS available latitude.}
 \item{lon}{the longitude. Values must be sequential and are rounded to the
 nearest GEFS available longitude.}
 
-\item{...}{additional parameters passed to \code{ncvar_get}}
+\item{...}{additional parameters passed to the connection.}
 
 \item{date}{A date/string formatted as YYYYMMDD.}
 
@@ -62,9 +68,23 @@ included between lat, lon, ens, and time.}
 \item{raw}{logical to indicate whether to return raw data matrix or
 reshaped data frame.}
 
-\item{con}{an ncdf4 connection.}
+\item{dim}{(character) the dimension to fetch values for.}
+
+\item{...}{connection parameters passed to \code{gefs_dimension_values()}.}
+
+\item{...}{connection parameters passed to \code{gefs_dimension_values()}.}
+
+\item{...}{connection parameters passed to \code{gefs_dimension_values()}.}
+
+\item{...}{connection parameters passed to \code{gefs_dimension_values()}.}
+
+\item{...}{connection parameters passed to \code{gefs_dimension_values()}.}
+
+\item{var}{(character) the variable for which to get dimensions.}
+
+\item{var}{(character) the variable for which to get dimension values for.}
 
-\item{dim}{(character) the dimension.}
+\item{...}{additional parameters.}
 }
 \value{
 a list containing metadata and accompanying data frame of
diff --git a/tests/testthat/test-gefs.R b/tests/testthat/test-gefs.R
index 1d8b84ab..233bd813 100644
--- a/tests/testthat/test-gefs.R
+++ b/tests/testthat/test-gefs.R
@@ -1,11 +1,16 @@
 context("gefs")
 
+# Get the temporary directory
+temp_dir <- dirname(tempdir())
+nc_temp_files <- list.files(temp_dir, pattern = "^occookie")
+
+
 #set a location
 lons <- c(-1.1, 0, 0.8, 2)
 lats <- c(50.1, 51, 51.9, 53, 54)
 
 #variable
-temp = "Temperature_height_above_ground_ens"
+temp <- "Temperature_height_above_ground_ens"
 
 test_that("gefs errors", {
   skip_on_cran()
@@ -116,59 +121,86 @@ test_that("gefs_variables returns characters.", {
   expect_is(vars[1], "character")
 })
 
-test_that("gefs_latitudes returns numeric.", {
+test_that("gefs_latitudes, gefs_longitudes, gefs_ensembless, gefs_times", {
   skip_on_cran()
   skip_on_travis()
   skip_on_appveyor()
   skip_on_os("windows")
 
-  lats = gefs_latitudes()
+  lats <-  gefs_latitudes()
   expect_is(lats, "array")
   expect_is(lats[1], "numeric")
-})
-
-test_that("gefs_longitudes returns numeric.", {
-  skip_on_cran()
-  skip_on_travis()
-  skip_on_appveyor()
-  skip_on_os("windows")
+  expect_equal(lats, array(90:-90))
 
-  lons = gefs_longitudes()
+  #vlats <- gefs_latitudes(var = temp)
+  
+  lons <-  gefs_longitudes()
   expect_is(lons, "array")
   expect_is(lons[1], "numeric")
+  expect_equal(lons, array(0:359))
+
+  #vlons <- gefs_longitudes(var = temp)
+
+  enss <-  gefs_ensembles()
+  expect_is(enss, "array")
+  expect_is(enss[1], "integer")
+  expect_equal(enss, array(0:20))
+
+  #venss <- gefs_ensembles(var = temp)
+
+  times <-  gefs_times()
+  expect_is(times, "array")
+  expect_is(times[1], "numeric")
+  expect_equal(times, array(seq(0,384, by = 6)))
+
+  #vlons <- gefs_times(var = temp)
 })
 
-test_that("gefs_dimensions returns character list.", {
+test_that("gefs_dimensions", {
   skip_on_cran()
   skip_on_travis()
   skip_on_appveyor()
   skip_on_os("windows")
 
-  dims = gefs_dimensions()
+  dims <- gefs_dimensions()
   expect_is(dims, "character")
   expect_is(dims[1], "character")
+
+  vdims <- gefs_dimensions(var = temp)
+  expect_true(all(vdims %in% c("lon", "lat", "height_above_ground", "ens",
+                               "time", "time1", "time2")))
 })
 
 test_that("gefs_dimension_values errors", {
   skip_on_os("windows")
+
   
-  expect_error(gefs_dimension_values(dim = "time2", var = temp),
-    "time2 is not in variable dimensions: lon, lat, height_above_ground, ens, time1.",
-    fixed = TRUE)
   expect_error(gefs_dimension_values(),
     "dim cannot be NULL or missing.", fixed = TRUE)
   expect_error(gefs_dimension_values(dim = "ens1"),
-    "ens1 is not a valid GEFS dimension. Check with 'gefs_dimensions()'.",
+    "ens1 is not a valid GEFS dimension. Get valid dimensions with 'gefs_dimensions()'.",
     fixed = TRUE)
+
+  tempdims <- gefs_dimensions(var = temp)
+  expect_error(gefs_dimension_values(dim = "isobaric3", var = temp),
+               paste0("isobaric3 is not in variable dimensions: ",
+                      paste0(tempdims, collapse = ", "),
+                      "."),
+               fixed = TRUE)
 })
 
-test_that("gefs_dimension_values returns numeric array.", {
+test_that("gefs_dimension_values", {
   skip_on_cran()
   skip_on_travis()
   skip_on_appveyor()
   skip_on_os("windows")
 
-  vals = gefs_dimension_values("lat")
+  vals <- gefs_dimension_values("lat")
   expect_is(vals, "array")
   expect_is(vals[1], "numeric")
 })
+
+test_that("no remaining connection temp files exist", {
+  nc_temp_files2 <- list.files(temp_dir, pattern = "^occookie")
+  expect_equal(nc_temp_files, nc_temp_files2)
+})