Create CalAdapt Download Functions for LOCA downscaled CMIP-5 #3446
Conversation
…, and factored out validation functionsdata(periods, package = "caladaptr", envir = environment())
dlebauer
changed the title
| #' out_dir = "data" | ||
| #' ) | ||
| #' } | ||
| download_caladapt_loca_raster <- function(sf_obj, |
There was a problem hiding this comment.
I get the reason for accepting a sf_obj instead of just lat/lon, which is non-standard for PEcAn met files, but beyond that I'd highly recommend doing everything else you an to adhere to workflow norms and patterns. These include naming the function download. and having the required inputs start with (outfolder, start_date, end_date), followed by spatial information, followed by product-specific variables. This order is also consistent with R norms of having variables that have defaults AFTER those that don't have a default (start/end). To that end, I'd recommend dropping the default on outfolder (what you call outdir) and renaming start_year and end_year to start_date and end_date
There was a problem hiding this comment.
Thanks for enumerating these, still getting back into the swing of things. This brings up a good point - it would be good to document these norms and patterns. I'm inclined to put it and existing code style etc docs into the CONTRIBUTING file so that it is all in one place. My only hesitation is that it could get very long. At the same time, this could be a good first step to focusing the documentation. Thoughts?
There was a problem hiding this comment.
It's too much into to put in the CONTRIBUTING file. The info should be in the main documentation, and if it's not then that should be updated. CONTRIBUTING should be short enough that folks are willing to read it in one push and should point to the main documentation for more detail (e.g., CONTRIBUTING could mention that many modules [met, models, SDA constraints, etc] have design patterns that enable scalability)
| period = period, | ||
| start_year = start_year, | ||
| end_year = end_year, | ||
| raster = rast_file |
There was a problem hiding this comment.
Are you planning on writing separate functions convert the raster to netCDF CF and extract info from it?
There was a problem hiding this comment.
yep download --> met2cf --> met2model
but next step is to write the download functions for the WRF downscaled CMIP-6. Hopefully (🤞🏻) met2cf.caladapt will work on both.
| #' out_dir = "data" | ||
| #' ) | ||
| #' coords <- data.frame(lat = 36, lon = -120) |> | ||
| #' st_as_sf(coords = c("lon", "lat"), crs = 4326) |
There was a problem hiding this comment.
should this be sf::st_as_sf, or maybe should there be a library call in the example?
| #' start_year = 2006, end_year = 2100, | ||
| #' out_dir = "data" | ||
| #' ) | ||
| #' } |
There was a problem hiding this comment.
?
| #' } | |
| #' } | |
| #' @export |
| if (!exists("gcms", envir = environment())) { | ||
| data(gcms, package = "caladaptr", envir = environment()) | ||
| } |
There was a problem hiding this comment.
| if (!exists("gcms", envir = environment())) { | |
| data(gcms, package = "caladaptr", envir = environment()) | |
| } | |
| gcms <- caladaptr::gcms |
There was a problem hiding this comment.
Same pattern for other functions that use caladaptr package data
There was a problem hiding this comment.
My suggestion does assume you want to always use the caladaptr version of the variable. If you're trying to allow users to provide their own overrides, I recommend making them be explicit function arguments rather than relying on specific object names being visible in scope.
Description
Adds
download_caladapt_locato download Cal-Adapt LOCA downscaled CMIP5 rasters inputs that can subsequently be used for data extraction. Acceptssf,sfc, andSpatVectorobjects.Motivation and Context
To support CCMMF project #3445
support for WRF-downscaled CMIP6 data will be added in a separate PR
Types of changes
Checklist: