Add functions to access GEFS ensemble forecasts. #106

potterzot · 2015-09-09T07:34:34Z

Hello,

This is my first pull request so I hope I am doing this correctly. Please let me know if I need to change something. I've tried to create the new files in the same format and style as existing rnoaa code. Here's the commit message:

Add functions to access GEFS ensemble forecasts.

Add R/gefs.R containing functionality to access GEFS forecasts and
several helper functions.
Add tests/testthat/test-gefs.R to test new gefs functions.
modify package files:
- DESCRIPTION now includes ncdf4 package in suggests,
- NAMESPACE exports gefs function,
- rnoaa-package.r updated to include reference to ncdf4

* Add R/gefs.R containing functionality to access GEFS forecasts and several helper functions. * Add tests/testthat/test-gefs.R to test new gefs functions. * modify package files: * DESCRIPTION now includes ncdf4 package in suggests, * NAMESPACE exports gefs function, * rnoaa-package.r updated to include reference to ncdf4

sckott · 2015-09-09T18:07:11Z

@potterzot Thanks for this! A few thoughts:

Do you need to use ncdf4 instead of ncdf? I'd rather go ncdf4, but ncdf is cross platform, so if you could get by with ncdf, that'd be better.
Please use a fxn to check whether user has ncdf/ncdf4 installed https://github.com/ropensci/rnoaa/blob/master/R/buoy.R#L186-L192 - Since it's in Suggests, they may not have the pkg installed
if you end up using ncdf4, it needs to be listed in the .travis.yml file like the ncdf eg https://github.com/ropensci/rnoaa/blob/master/.travis.yml#L5
There's no manual file for gefs, should be added if you document()/roxygenise()
As far as I know, you can't use importFrom/etc. with packages in Suggests in a package. So just remove those lines and redocument.
Some comments inline on the commit as well

you can make commits and push to this same PR, changes will be added here

sckott · 2015-09-09T18:07:23Z

DESCRIPTION

@@ -33,10 +33,11 @@ Imports:
    scales,
    rgdal,
    XML,
-    jsonlite
+    jsonlite,


remove trailing comma

Changes include: * switch from ncdf4 to ncdf * add function check for ncdf (from bouy.R) * generate documentation and namespace exports for gefs functions * roll back rnoaa-package doc to remove ncdf4 references * include link to gefs web page in documentation * include skip_on_cran() where tests make a web api call * minor fixes (trailing commas, etc...)

potterzot · 2015-09-09T20:32:17Z

@sckott Thanks for your comments. Update should address all of these. Switched over to ncdf, etc...

Two commits, the second to call ncdf functions like ncdf::open.ncdf() rather than open.ncdf() so that tests are passed. Without the @importFrom the tests fail. If there's a better way to do that let me know.

sckott · 2015-09-09T21:22:58Z

@potterzot thanks!

the second to call ncdf functions like ncdf::open.ncdf() rather than open.ncdf()

Yep, that's what I do https://github.com/ropensci/rnoaa/blob/master/R/buoy.R#L127

sckott · 2015-09-09T21:45:09Z

R/gefs.R

+#' # here ensembles 1-3 (ensembles are numbered starting with 0)
+#' # and time for 2 days from today at 1800
+#' var = "Temperature_height_above_ground_ens"
+#' gefs(var, lat, lon, forecast_time = "1800", ens_idx=2:4, time_idx=1:8)


@potterzot This eg doesn't work for me, thoughts?

gefs(var, lat, lon, forecast_time = "1800", ens_idx=2:4, time_idx=1:8) #> Error in R_nc_open: NetCDF: Can't read file #> Error in ncdf::open.ncdf(gefs_url) : #> Error in open.ncdf trying to open file #> http://motherlode.ucar.edu/thredds/dodsC/grib/NCEP/GEFS/Global_1p0deg_Ensemble/members/GEFS_Global_1p0deg_Ensemble_20150909_1800.grib2

i think that URL doesn't exist, perhaps the URL is not being built correctly?

Ah, yes, It's not yet 1800 UTC so it fails. I will rewrite to add a date that is the previous day so that it will work regardless of the hour that someone runs it.

If you try `forecast_time = "0600" for ex., it should work. Will push an update in a sec.

great, thanks, guess I should have caught that

Add date for previous day so that the example may be run at any time. Previously the example could give an error if the 1800UTC forecast link was not up yet.

sckott · 2015-09-10T17:34:17Z

thanks, this is awesome @potterzot

Add functions to access GEFS ensemble forecasts.

potterzot · 2015-09-10T19:11:42Z

Awesome!

I'd like to add acknowledgement for a professor that wrote some python code
that I used as a reference. Is that possible and if so what would be the
best way to do it?

I had pushed a change to gefs.R, but I think I pushed it to my fork's
master branch. Here is the change:

#' @references Data description: \url{
https://www.ncdc.noaa.gov/data-access/model-data/model-datasets/global-ensemble-forecast-system-gefs
}.
#' Adapted from Python code written by Von P. Walden, Washington State
University: \url{
http://sila.cee.wsu.edu/forecasts/WeatherAndClimateDatafromNWS.html}

Nicholas Potter
http://potterzot.com

On Thu, Sep 10, 2015 at 10:34 AM, Scott Chamberlain <
notifications@github.com> wrote:

Merged #106 #106.

—
Reply to this email directly or view it on GitHub
#106 (comment).

sckott · 2015-09-10T19:20:02Z

Yes, we can add that. If okay with you, i made a bit.ly link out of that GEFS reference link as its longer than 100 characters, and CRAN check doesn't like that. The link: http://bit.ly/noaagefs

potterzot · 2015-09-10T19:22:05Z

Sounds great. Thanks for your help!

sckott reviewed Sep 9, 2015
View reviewed changes

DESCRIPTION

@@ -33,10 +33,11 @@ Imports:

scales,

rgdal,

XML,

jsonlite

jsonlite,

Copy link

Contributor

sckott Sep 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove trailing comma

potterzot added 2 commits September 9, 2015 12:56

add hard reference to ncdf functions to pass tests.

7735e56

sckott reviewed Sep 9, 2015
View reviewed changes

fix example that can give error if 1800UTC forecast is not up yet.

243e76a

Add date for previous day so that the example may be run at any time. Previously the example could give an error if the 1800UTC forecast link was not up yet.

sckott added a commit that referenced this pull request Sep 10, 2015

Merge pull request #106 from potterzot/gefs

d5aba66

Add functions to access GEFS ensemble forecasts.

sckott merged commit d5aba66 into ropensci:master Sep 10, 2015

sckott added this to the v0.5 milestone Sep 10, 2015

potterzot deleted the gefs branch September 10, 2015 19:21

sckott added a commit that referenced this pull request Sep 10, 2015

fix to gefs documentation, #106

dbf76db

potterzot mentioned this pull request Nov 30, 2015

GEFS Forecast data #105

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add functions to access GEFS ensemble forecasts. #106

Add functions to access GEFS ensemble forecasts. #106

potterzot commented Sep 9, 2015

sckott commented Sep 9, 2015

sckott Sep 9, 2015

potterzot commented Sep 9, 2015

sckott commented Sep 9, 2015

sckott Sep 9, 2015

potterzot Sep 9, 2015

sckott Sep 9, 2015

sckott commented Sep 10, 2015

potterzot commented Sep 10, 2015

sckott commented Sep 10, 2015

potterzot commented Sep 10, 2015

Add functions to access GEFS ensemble forecasts. #106

Add functions to access GEFS ensemble forecasts. #106

Conversation

potterzot commented Sep 9, 2015

sckott commented Sep 9, 2015

sckott Sep 9, 2015

Choose a reason for hiding this comment

potterzot commented Sep 9, 2015

sckott commented Sep 9, 2015

sckott Sep 9, 2015

Choose a reason for hiding this comment

potterzot Sep 9, 2015

Choose a reason for hiding this comment

sckott Sep 9, 2015

Choose a reason for hiding this comment

sckott commented Sep 10, 2015

potterzot commented Sep 10, 2015

sckott commented Sep 10, 2015

potterzot commented Sep 10, 2015