MazamaScience / MazamaSatelliteUtils

Satellite Data Download and Utility Functions
http://mazamascience.github.io/MazamaSatelliteUtils
GNU Lesser General Public License v3.0
0 stars 0 forks source link

Refactor 2: Harmonize #78

Closed tabrasel closed 3 years ago

tabrasel commented 3 years ago

Harmonize MazamaSatelliteUtils function code and documentation style with other Mazama Science packages. Ignore maiac_~ functions for now.

tabrasel commented 3 years ago

Non-maiac_~() package functions now have more clean and consistent structure (code formatting/organization) and documentation (roxygen docs and comments). I made no changes to actual function behavior, but I did make one minor refactor to rename endTime params to endtime. I'm still debating that choice though. While I can see that "datetime" has sort of become its own word and "end date" is two, using datetime and endTime throughout the package can be a little confusing with their tiny capitalization difference. Also, endTime is really just another datetime, so it might be more appropriate to rename it to endDatetime instead. Splitting hairs, like usual! More serious behavioral changes will be saved for an upcoming refactor.

The video executable script and testthat tests are all that are left to harmonize.

jonathancallahan commented 3 years ago

I have gone back and forth on naming of date and time variables so often. 🤯

Reviewing all of the AirFire code with lines like grep "startTime <-" */R/* | sort -u I see that lower case is more common.

Let's make an executive decision that we won't capitalize any of these. So our code base should always have:

This also matches my preference that web service APIs avoid capitalization.

tabrasel commented 3 years ago

One last minor change: The ncList param in goesaodc_areaPlot()'s is now just nc. This seems more fitting since the function should allow users to pass in a single handle as opposed to having to put it in a list first (areaPlot(nc) vs. areaPlot(list(nc))). It also matches other functions that already use an nc param, such as goesaodc_createNativeGrid. However, goesaodc_areaPlot() currently requires you to pass in a list. So my next refactor (actually changing function behavior) will patch in the following code from goesaodc_createNativeGrid to force nc into a list in the background:

if ( "list" %in% class(nc) ) {
  ncList <- nc
} else if ( "ncdf4" %in% class(nc) ) {
  ncList <- list(nc)
} else {
  stop("Parameter 'nc' must be of type 'list' or 'ncdf4'")
}