Provide reference dates in download.CRUNCEP output #1357
Conversation
Caution: slow, requires ful-year download from thredds server. Should this be mocked out? Skipped on Travis?
There was a problem hiding this comment.
Looks good to me but will defer to authors @jsimkins2 or @mdietze for review / approval
| time <- ncdf4::ncdim_def(name = "time", units = "sec", vals = (1:ntime) * 21600, | ||
| create_dimvar = TRUE, unlim = TRUE) | ||
|
|
||
| timestamps <- (1:ntime) * 6/24 - 3/24 # data are 6-hourly, with timestamp at center of interval |
There was a problem hiding this comment.
So are you keeping the timestamp? Shifting time? What are time stamps of input and output data?
There was a problem hiding this comment.
We're keeping the timestamp. Or more precisely, we're generating a completely new time series here, then on reading each variable in the loop below we check that input times have the same values after converting to our output units.
Input timestamps of CRUNCEP nc files appear to always have units "days since 1700-01-01T00:00:00Z", so a file starting 2004-01-01 looks like 111033.1 111033.4 111033.6 111033.9 111034.1 .... Output timestamps are days since start_date, so the times always look like 0.125 0.375 0.625 0.875 1.125 ....
| stopifnot(dap$dim$time$len == ntime) | ||
| dap_time <- udunits2::ud.convert(dap$dim$time$vals, | ||
| dap$dim$time$units, | ||
| paste0("days since ", start_year, "-01-01")) |
There was a problem hiding this comment.
Awesome that is.units can parse this
There was a problem hiding this comment.
Right? I should avoid ambiguity and add "00:00:00Z" to the output units, though.
infotroph
changed the title
| @@ -79,6 +99,14 @@ download.CRUNCEP <- function(outfolder, start_date, end_date, site_id, lat.in, l | |||
| # This throws an error if file not found | |||
| dap <- ncdf4::nc_open(dap_file) | |||
|
|
|||
| # confirm that timestamps match | |||
| stopifnot(dap$dim$time$len == ntime) | |||
There was a problem hiding this comment.
Would prefer if you use PEcAn's logger functions so that we can ensure that this stop is properly caught and written out to the log file.
|
Note that setting up a test that depends on downloading from CRUNCEP will occasionally cause the build to fail when the code is fine if the connection to CRUNCEP is lost. |
Description
Ensures output times have CF-compliant units (#1347).
Also some code cleanup while I'm at it:
overwriteparameter (previously ignored)For discussion
The test I give here is slow! It downloads a full year of CRUNCEP data to do one check (so far) on the output. Is this a problem? If so, should the download be mocked out, or the test skipped in some environments?
Types of changes
Checklist: