Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch vpts data from aloft #9

Open
24 of 27 tasks
PietrH opened this issue Nov 12, 2024 · 6 comments · May be fixed by #10
Open
24 of 27 tasks

Fetch vpts data from aloft #9

PietrH opened this issue Nov 12, 2024 · 6 comments · May be fixed by #10
Assignees
Labels
enhancement New feature or request

Comments

@PietrH
Copy link
Contributor

PietrH commented Nov 12, 2024

  • Needs to support fetching multiple radars as a character vector
  • Needs to support a single date as input
  • Needs to support date interval as input
  • Fetch from https://aloftdata.s3-eu-west-1.amazonaws.com in parallel,
  • should we provide an option to set a different aws region?
  • add user agent to requests to s3
  • compare to coverage to figure out what data is available
  • Allow users to provide intervals with a resolution smaller than a single day
  • Add a column to the returned data.frame with the radar source
  • Add source argument
  • return object compatible with {bioRad}
  • also allow returning tibble with source column: as_vpts = TRUE argument?
  • set readr spec/cols for read_csv to be more defensive and efficient!
  • R CMD CHECK
  • update NEWS
  • up dev version number
  • update DESCRIPTION
  • split tests to do with vpts output object testing
  • rename as_vpts to as_tibble and set behaviour as inverse
  • move helpers to utils.R
  • abstract fetching csv from web in parallel with user agent to helper
  • ❗ ❗ fix bug in bioRad:::vpts_validate() call ❗ ❗
  • rename arguments in get_vpts_aloft() to be in line with get_vpts() as much as possible
  • pass R CMD CHECK on CI
  • rename get_aloft_coverage() to aloft_coverage()
  • set aloft_coverage() to public function
  • create general file getter helper instead of read_vpts_from_url(): use for all functions that get multiple files from web

If a radar is present in both UVA and BALTRAD, both are fetched.

@PietrH PietrH self-assigned this Nov 12, 2024
@PietrH PietrH added the enhancement New feature or request label Nov 12, 2024
@bart1
Copy link
Collaborator

bart1 commented Nov 13, 2024

How about using col_types for read_csv and specifying some columns as factors (saves memory). That will directly also so some data type checking

@bart1
Copy link
Collaborator

bart1 commented Nov 13, 2024

I would also consider using glue over reprex magick :), its a dependency anyway and much easier to understand

@PietrH
Copy link
Contributor Author

PietrH commented Nov 13, 2024

There is now a progress bar with an ETA for larger intervals, eg:

get_vpts("bejab", lubridate::interval("20210101",20210301"))

@PietrH
Copy link
Contributor Author

PietrH commented Nov 13, 2024

vpts objects in bioRad can only contain data of a single radar, how do we want to handle that? via bioRad::as.vpts()

@PietrH
Copy link
Contributor Author

PietrH commented Nov 13, 2024

vpts objects in bioRad can only contain data of a single radar, how do we want to handle that? via bioRad::as.vpts()

I've decided to return a list of vpts objects

@PietrH PietrH linked a pull request Nov 13, 2024 that will close this issue
@peterdesmet
Copy link
Member

peterdesmet commented Nov 18, 2024

I discussed the get_vpts() function with @CeciliaNilsson709 and here's what we find the most straightforward:

  • source is required parameter
  • source only accepts one value. This is in line with get_pvol(). Downloading from multiple sources in one function call adds unnecessary complexity.
  • source can have baltrad as default
  • Return the data as a list of vpts objects
  • If only one radar was chosen, return as a single vpts object
  • When a list, use the provided radar code (not the one in the data) as identifier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants