Closed tabrasel closed 3 years ago
Refactors for existing functions are now complete. Their surface level behavior is mostly the same, but now includes a few additional features & niceties like param validation, proper param passing, verbose printing, standardized bboxes, etc.
The main refactor I made that did change behavior though was editing the endtime
param in goesaodc_listFiles()
to be exclusive. Previously, this function would use a time range specified by: datetime <= x <= endtime
. My refactor changes this so the time range is: datetime <= x < endtime
, which makes endtime
act like a more traditional end boundary. Maybe in the future we could include a ceilingEnd=TRUE/FALSE
param like MazamaCoreUtils:timeRange()
, but for now that's probably not necessary.
During the harmonization refactor, I noticed a few functions could use some minor fixes/improvements that will (or could potentially) change their behavior or how they are used:
FUNCTION REFACTORS:
getDaylightHours
bbox
default and documentation. Despite the param doc stating that it defaults to CONUS, the param is set to null. The function is fairly location specific--so it's probably better thatbbox
defaults to null. I'll change the docs.goesaodc_areaPlot
...
param is documented as "Additional arguments passed to \code{goesaodc_plotSpatialPoints()}." but is never actually passed. Fix this so it is!nc
is a single ncdf4 handle, put it into a single element list (likegoesaodc_createNativeGrid()
).goesaodc_createNativeGrid
verbose
param is never used. Not sure what information should be printed out here...maybe it wasn't supposed to have the param in the first place? I see this function callsncdf4::ncvar_get(..., verbose=FALSE, ...)
several times, so I suppose I could replace those withncdf4::ncvar_get(..., verbose=verbose, ...)
, but that spits out a crazy amount of of information which doesn't even fit in the R console. I think I will get rid of theverbose
param for now.goesaodc_createRasterStack
stacked layer (12/58): ...
.bbox
to thebbox_CONUS
package variable instead of defining its own vector.goesaodc_createSpatialPoints
bbox
to thebbox_CONUS
package variable instead of defining its own vector.goesaodc_downloaddaytimeAOD
bbox
default and documentation. Despite the param doc stating that it defaults to CONUS, the param is set to null. I think it makes since for the param to default to null, so I'll update the docs.goesaodc_listDaytimeFiles
bbox
default and documentation. Despite the param doc stating that it defaults to CONUS, the param is set to null. I think it makes since for the param to default to null, so I'll update the docs.baseUrl
param that is passed to the internalgoesaodc_listFiles()
call.goesaodc_listFiles
datetime
andendtime
.enddate
is beforestartdate
.endtime
should be the first time excluded from the end of the time range.goesaodc_plotSpatialPoints
raster_createLocationTimeseries
raster_createStackAverage
utils_raster.R
.utils_goesaodc.R
GENERAL REFACTORS:
Validation of datetime types
goesaodc_createRasterStack()
andgoesaodc_createDaytimeRasterStack()
validate datetime/enddtime with:while
goesaodc_ListFiles()
uses:Are they the same? Is one preferable to the other? I like the lubridate version for its readability, but lubridate is only used 5 times throughout our package and it seems like overkill to import if we use it so little. Since the lubridate version seems more clean and "safe" though, I'm going to switch functions to use that for now.
Verbose display
message()
instead ofprint()
.