Add support for Swedish radar data (fix #96) - #97
Conversation
bart1
changed the title
|
Just some quick questions, I didn't do a full review. I can come back to this next week. |
| httr2::req_url_path_append( | ||
| glue::glue(url) | ||
| ) |> | ||
| httr2::req_perform(path = tempfile(fileext = ".h5"), error_call = call), |
There was a problem hiding this comment.
Do we want to req_retry_getrad() or req_cache_getrad()?
Do we report progress when interactive for large files?
There was a problem hiding this comment.
I think we should not cache full pvol file, they are large and bulky and will make the cache run full too quickly. The retry I would only do when we notice frequent timeouts
There was a problem hiding this comment.
Agreed on the caching.
What is the added cost of adding the retry? I think it would be nice if someone were to run this on a less reliable connection.
There was a problem hiding this comment.
For retry I guess it could be done, risk being to unnecessarily delay the responses that are erroring. Without experience it is hard to say if certain errors do frequently occur and if they are resolved by retries. If your connection is really unstable downloading large files is maybe anyway not what you want to do.
There was a problem hiding this comment.
I want to be inclusive of those who don't have a choice in the stability of their connection. If an error is permanent, then a retry would not add a lot of time since we are only fetching a single file in this case?
If you are envisioning users fetching a large number of files where the delay of possible retries is a problem, we should add package options to change the default retry behaviour so users can control it (or even just turn it off).
There was a problem hiding this comment.
I guess retries could be added, I guess I'm a bit more of the school of trying to avoid to many complications that do not solve real world problems that have been encountered :). For pipelines you would anyway implement some retries, a few hours later for error that are transient on other timescales
| ) | ||
| } | ||
| radar_name <- get_pvol_se_radar_mapping[radar] | ||
| url <- glue::glue('/area/{radar_name}/product/qcvol/{lubridate::year(time)}/{lubridate::month(time)}/{{lubridate::day(time)}}/radar_{radar_name}_qcvol_{strftime(time, "%Y%m%d%H%M", tz="UTC" )}.h5') |
There was a problem hiding this comment.
I'd probably break this long string up with req_url_path_append(), file.path (Peter likes this one, I try to avoid it), or usage of glue .sep:
eg.
glue::glue("area","bewid","product","qcvol",2018,"04",16,"radar_bewid_qcvol_040417.h5", .sep = "/")
``There was a problem hiding this comment.
That looks like a re-implementation of paste ;), I guess I do like the longer format strings as then it can be controlled through a package option :)
There was a problem hiding this comment.
Implemented options for control
|
Would be good if @CeciliaNilsson709 and/or Johan test this new functionality. @bart1, can you provide a quick example? |
Co-authored-by: Peter Desmet <peter.desmet@inbo.be>
There is an example in #96, @CeciliaNilsson709 do you plan to test this soon? otherwise I will just merge |
|
I'll take a quick look at it now! |
|
Seems to work fine for me, tried a few different radars and works for the last 24 hours. I don't get as many parameters though (DBZH, VRADH, TH and WRADH) so not the CCORH we discussed in #96 . Haven't checked with SMHI about the qcvol get, will get back to you on that! |
|
@CeciliaNilsson709 Thanks for checking! devtools::load_all("~/getRad")
#> ℹ Loading getRad
bioRad::get_scan(get_pvol("seoer", Sys.time()-700),1)
#> Polar scan (class scan)
#>
#> parameters: DBZH VRADH TH WRADH
#> elevation angle: 1 deg
#> dims: 480 bins x 360 rays
bioRad::get_scan(get_pvol("seoer", Sys.time()-700, param="all"),1)
#> Polar scan (class scan)
#>
#> parameters: DBZH UVRADH CI SQIH CCORH VRADH TH LDR UPSPH CPA WRADH CID VRADDH USNRHC ULDR
#> elevation angle: 1 deg
#> dims: 480 bins x 360 rays |
|
The check error was related to to many request at the dutch repository |
4 participants
AI Summary
This pull request introduces support for downloading Swedish radar data, including a new function, tests, and documentation updates. The changes ensure compatibility with Swedish data sources and provide error handling for common issues.
Support for Swedish Radar Data:
get_pvol_seinR/get_pvol_se.Rto handle downloading and processing radar data from Sweden, including a radar mapping and error handling for invalid radar codes and unavailable data.select_get_pvol_functioninR/get_pvol.Rto include Sweden (cntry_code == "se") in the country-specific radar data retrieval logic.Testing:
tests/testthat/test-get_pvol_se.Rto verify successful data retrieval for Sweden, proper handling of time range errors, and validation of radar codes.Documentation:
NEWS.mdto announce support for downloading Swedish data.vignettes/supported_sources.Rmdto include Sweden in the list of supported countries, with details on data availability and licensing.