add `get_pvol` #1

bart1 · 2024-10-14T16:06:03Z

This pull request implements downloads of polar volumes for dk, nl, de, cz, ee & fi

It implements some basic testing to see if it works, however as tests are dependent on authentication and stable connections not all tests will always run.

I have kept most dependencies as suggests. One I doubted about was bioRad as it adds a lot of other dependencies. But as it is core to pvol I kept is in import

bart1 · 2024-10-14T16:17:46Z

Maybe a few more notes on some design:

I try to consistently set the user agent so requests through the package can be tracked. Would it be useful to add the package version number or commit here? If so how to keep that up to date.
I now class each error as "getRad_error_...." we could also go for c("getRad_error_....","getRad_error") more like httr2
currently time should be a POSIXct rounded to 5 minutes we might consider also allowing for intervals and multiple timestamps. The individual functions now only download one file at a time and do not parallelize requests.

peterdesmet · 2024-10-14T16:23:35Z

Can you list your design notes as issues? Easier to answer and track.

Longer term we’ll have to be careful not to introduce circular dependencies between this and bioRad

codecov · 2024-10-14T18:47:04Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

bart1 · 2024-10-14T19:00:45Z

Can you list your design notes as issues? Easier to answer and track.

See #2, #3 & #4

bart1 · 2024-10-15T10:08:49Z

I have also added a bit of examples to the readme

bart1 · 2024-10-16T09:37:40Z

@peterdesmet I think this is quite a coherent set of functions that can be added/reviewed/tested

Prefer using TRUE over T in logical values as per tidyverse style guide: https://style.tidyverse.org/syntax.html#logical-vectors In principle T could be reassigned in the global environment, so stick to TRUE everywhere for consistency

puts fields in standard order and alphabetises dependencies.

PietrH · 2024-10-28T12:49:57Z

I'm currently failing a few tests locally, looking into it:

── Failed tests ────────────────────────────────────────────────────────────
Failure (test-get_pvol.R:32:3): multiple timestamps work
lapply(pvl, function(x) x$datetime) (`actual`) not identical to as.list(t) (`expected`).

`attr(actual[[1]], 'tzone')` is a character vector ('UTC')
`attr(expected[[1]], 'tzone')` is absent

`attr(actual[[2]], 'tzone')` is a character vector ('UTC')
`attr(expected[[2]], 'tzone')` is absent

Failure (test-get_pvol.R:40:3): multiple timestamps and radars work
lapply(pvl, function(x) x$datetime) (`actual`) not identical to as.list(rep(t, each = 2)) (`expected`).

`attr(actual[[1]], 'tzone')` is a character vector ('UTC')
`attr(expected[[1]], 'tzone')` is absent

`attr(actual[[2]], 'tzone')` is a character vector ('UTC')
`attr(expected[[2]], 'tzone')` is absent

`attr(actual[[3]], 'tzone')` is a character vector ('UTC')
`attr(expected[[3]], 'tzone')` is absent

`attr(actual[[4]], 'tzone')` is a character vector ('UTC')
`attr(expected[[4]], 'tzone')` is absent

PietrH · 2024-10-28T13:05:01Z

The minimum required R version of this package is 4.1.0 because of use of the new syntax for anonymous functions: \(x), this should be specified in DESCRIPTION

The package also uses R native pipes |>, but has access to magrittr pipes via a dependency on dplyr.

We could support lower R versions if we wanted to. I believe tidyverse will mostly drop it's minimum supported R version to R 4.1 in spring 2025 anyway: https://www.tidyverse.org/blog/2019/04/r-version-support/

Depends: R (>= 4.1.0)

See aa5a85e

…tions

Avoid assignment in function calls! https://style.tidyverse.org/syntax.html#assignment

See also rlang::is_list(), avoids some inconsistency with S3/S4

bart1 · 2024-10-28T13:53:05Z

I'm currently failing a few tests locally, looking into it:

── Failed tests ────────────────────────────────────────────────────────────
Failure (test-get_pvol.R:32:3): multiple timestamps work
lapply(pvl, function(x) x$datetime) (`actual`) not identical to as.list(t) (`expected`).

`attr(actual[[1]], 'tzone')` is a character vector ('UTC')
`attr(expected[[1]], 'tzone')` is absent

`attr(actual[[2]], 'tzone')` is a character vector ('UTC')
`attr(expected[[2]], 'tzone')` is absent

Failure (test-get_pvol.R:40:3): multiple timestamps and radars work
lapply(pvl, function(x) x$datetime) (`actual`) not identical to as.list(rep(t, each = 2)) (`expected`).

`attr(actual[[1]], 'tzone')` is a character vector ('UTC')
`attr(expected[[1]], 'tzone')` is absent

`attr(actual[[2]], 'tzone')` is a character vector ('UTC')
`attr(expected[[2]], 'tzone')` is absent

`attr(actual[[3]], 'tzone')` is a character vector ('UTC')
`attr(expected[[3]], 'tzone')` is absent

`attr(actual[[4]], 'tzone')` is a character vector ('UTC')
`attr(expected[[4]], 'tzone')` is absent

Hmm interesting it looks like this relates to hoe expectations are generated , for me all these example have the tzone attribute. Could you check on your computer:

as.POSIXct(Sys.Date()) |> dput()
#> structure(1730073600, class = c("POSIXct", "POSIXt"), tzone = "UTC")
(c(-300, -900) + as.POSIXct(Sys.Date())) |> dput()
#> structure(c(1730073300, 1730072700), class = c("POSIXct", "POSIXt"
#> ), tzone = "UTC")
rep(c(-300, -900) + as.POSIXct(Sys.Date()), each=TRUE) |> dput()
#> structure(c(1730073300, 1730072700), class = c("POSIXct", "POSIXt"
#> ), tzone = "UTC")

sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.4.0 (2024-04-24)
#>  os       Ubuntu 22.04.4 LTS
#>  system   x86_64, linux-gnu
#>  ui       X11
#>  language (EN)
#>  collate  en_US.UTF-8
#>  ctype    en_US.UTF-8
#>  tz       Europe/Amsterdam
#>  date     2024-10-28
#>  pandoc   3.1.11 @ /usr/lib/rstudio/resources/app/bin/quarto/bin/tools/x86_64/ (via rmarkdown)
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version date (UTC) lib source
#>  cli           3.6.3   2024-06-21 [1] CRAN (R 4.4.0)
#>  digest        0.6.35  2024-03-11 [1] CRAN (R 4.4.0)
#>  evaluate      0.23    2023-11-01 [1] CRAN (R 4.4.0)
#>  fastmap       1.2.0   2024-05-15 [1] CRAN (R 4.4.0)
#>  fs            1.6.4   2024-04-25 [1] CRAN (R 4.4.0)
#>  glue          1.8.0   2024-09-30 [1] CRAN (R 4.4.0)
#>  htmltools     0.5.8.1 2024-04-04 [1] CRAN (R 4.4.0)
#>  knitr         1.46    2024-04-06 [1] CRAN (R 4.4.0)
#>  lifecycle     1.0.4   2023-11-07 [1] CRAN (R 4.4.0)
#>  magrittr      2.0.3   2022-03-30 [1] CRAN (R 4.4.0)
#>  purrr         1.0.2   2023-08-10 [1] CRAN (R 4.4.0)
#>  R.cache       0.16.0  2022-07-21 [1] CRAN (R 4.4.0)
#>  R.methodsS3   1.8.2   2022-06-13 [1] CRAN (R 4.4.0)
#>  R.oo          1.26.0  2024-01-24 [1] CRAN (R 4.4.0)
#>  R.utils       2.12.3  2023-11-18 [1] CRAN (R 4.4.0)
#>  reprex        2.1.0   2024-01-11 [1] CRAN (R 4.4.0)
#>  rlang         1.1.4   2024-06-04 [1] CRAN (R 4.4.0)
#>  rmarkdown     2.26    2024-03-05 [1] CRAN (R 4.4.0)
#>  rstudioapi    0.16.0  2024-03-24 [1] CRAN (R 4.4.0)
#>  sessioninfo   1.2.2   2021-12-06 [1] CRAN (R 4.4.0)
#>  styler        1.10.3  2024-04-07 [1] CRAN (R 4.4.0)
#>  vctrs         0.6.5   2023-12-01 [1] CRAN (R 4.4.0)
#>  withr         3.0.1   2024-07-31 [1] CRAN (R 4.4.0)
#>  xfun          0.43    2024-03-25 [1] CRAN (R 4.4.0)
#>  yaml          2.3.9   2024-07-05 [1] CRAN (R 4.4.0)
#> 
#>  [1] /home/bart/R/x86_64-pc-linux-gnu-library/4.4
#>  [2] /usr/local/lib/R/site-library
#>  [3] /usr/lib/R/site-library
#>  [4] /usr/lib/R/library
#> 
#> ──────────────────────────────────────────────────────────────────────────────

PietrH · 2024-10-28T13:57:16Z

Indeed, I'm missing tzone

as.POSIXct(Sys.Date()) |> dput()
#> structure(1730073600, class = c("POSIXct", "POSIXt"))

(c(-300, -900) + as.POSIXct(Sys.Date())) |> dput()
#> structure(c(1730073300, 1730072700), class = c("POSIXct", "POSIXt"
#> ))

rep(c(-300, -900) + as.POSIXct(Sys.Date()), each=TRUE) |> dput()
#> structure(c(1730073300, 1730072700), class = c("POSIXct", "POSIXt"
#> ))

sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.2.1 (2022-06-23)
#>  os       Ubuntu 22.10
#>  system   x86_64, linux-gnu
#>  ui       X11
#>  language en_US
#>  collate  en_US.UTF-8
#>  ctype    en_US.UTF-8
#>  tz       Europe/Brussels
#>  date     2024-10-28
#>  pandoc   3.1.11 @ /usr/lib/rstudio/resources/app/bin/quarto/bin/tools/x86_64/ (via rmarkdown)
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version date (UTC) lib source
#>  cli           3.6.3   2024-06-21 [1] CRAN (R 4.2.1)
#>  digest        0.6.36  2024-06-23 [1] CRAN (R 4.2.1)
#>  evaluate      0.24.0  2024-06-10 [1] CRAN (R 4.2.1)
#>  fastmap       1.2.0   2024-05-15 [1] CRAN (R 4.2.1)
#>  fs            1.6.4   2024-04-25 [1] CRAN (R 4.2.1)
#>  glue          1.7.0   2024-01-09 [1] CRAN (R 4.2.1)
#>  htmltools     0.5.8.1 2024-04-04 [1] CRAN (R 4.2.1)
#>  knitr         1.47    2024-05-29 [1] CRAN (R 4.2.1)
#>  lifecycle     1.0.4   2023-11-07 [1] CRAN (R 4.2.1)
#>  magrittr      2.0.3   2022-03-30 [1] CRAN (R 4.2.1)
#>  purrr         1.0.2   2023-08-10 [1] CRAN (R 4.2.1)
#>  R.cache       0.16.0  2022-07-21 [1] CRAN (R 4.2.1)
#>  R.methodsS3   1.8.2   2022-06-13 [1] CRAN (R 4.2.1)
#>  R.oo          1.26.0  2024-01-24 [1] CRAN (R 4.2.1)
#>  R.utils       2.12.3  2023-11-18 [1] CRAN (R 4.2.1)
#>  reprex        2.1.0   2024-01-11 [1] CRAN (R 4.2.1)
#>  rlang         1.1.4   2024-06-04 [1] CRAN (R 4.2.1)
#>  rmarkdown     2.27    2024-05-17 [1] CRAN (R 4.2.1)
#>  rstudioapi    0.16.0  2024-03-24 [1] CRAN (R 4.2.1)
#>  sessioninfo   1.2.2   2021-12-06 [1] CRAN (R 4.2.1)
#>  styler        1.10.3  2024-04-07 [1] CRAN (R 4.2.1)
#>  vctrs         0.6.5   2023-12-01 [1] CRAN (R 4.2.1)
#>  withr         3.0.1   2024-07-31 [1] CRAN (R 4.2.1)
#>  xfun          0.45    2024-06-16 [1] CRAN (R 4.2.1)
#>  yaml          2.3.10  2024-07-26 [1] CRAN (R 4.2.1)
#> 
#>  [1] /home/pieter_huybrechts/R/x86_64-pc-linux-gnu-library/4.2
#>  [2] /usr/local/lib/R/site-library
#>  [3] /usr/lib/R/site-library
#>  [4] /usr/lib/R/library
#> 
#> ──────────────────────────────────────────────────────────────────────────────

^{Created on 2024-10-28 with reprex v2.1.0}

bart1 · 2024-10-28T14:03:07Z

Intersting appearently a change in how as.POSIXct works, most likely in R 4.3.0:

as.POSIXlt(<Date>) now does apply a tz (time zone) argument, as does as.POSIXct(); partly suggested by Roland Fuß on the R-devel mailing list.

https://cran.r-project.org/doc/manuals/r-release/NEWS.html
Maybe it is best if you can just add a tz argument to the expectation and see if it works on your computer?

PietrH · 2024-10-28T14:39:34Z

Intersting appearently a change in how as.POSIXct works, most likely in R 4.3.0:
as.POSIXlt(<Date>) now does apply a tz (time zone) argument, as does as.POSIXct(); partly suggested by Roland Fuß on the R-devel mailing list.
https://cran.r-project.org/doc/manuals/r-release/NEWS.html Maybe it is best if you can just add a tz argument to the expectation and see if it works on your computer?

Having considered it, I think we should avoid the Sys.Date() call altogether and use lubridate::ymd_hms() with an arbitrary date instead. I suppose something like lubridate::now() would also work.

bart1 · 2024-10-28T14:42:13Z

Having considered it, I think we should avoid the Sys.Date() call altogether and use lubridate::ymd_hms() with an arbitrary date instead. I suppose something like lubridate::now() would also work.

now could work but the date needs to be in the last few days as German data is not public for more then 3 days ....

PietrH · 2024-10-28T14:59:00Z

Having considered it, I think we should avoid the Sys.Date() call altogether and use lubridate::ymd_hms() with an arbitrary date instead. I suppose something like lubridate::now() would also work.

now could work but the date needs to be in the last few days as German data is not public for more then 3 days ....

~~Right, I'll make a suggestion~~ -> I've changed it directly on the branch; used lubridate::ymd_hms() in combination with lubridate::today()

I prefer doing it in this order because it allows me to run the expectations in any order, it also allows us to change/remove earlier expectations without having to make changes to later expectations

…efault before R4.3

…tead of expect_true(is.list : easier to read, run expectations out of order

Bart added 12 commits October 14, 2024 11:42

add secrets scaffolding

ccd734e

add get_pvol_fi

556afdc

add cz

acfcdbb

add de

74ab345

add nl and ee

bf92239

add dk

88565c9

converter for nl and examples get_pvol

29136d5

add vignette

f4ba645

solve global vars

3157c6d

move file work on checks

8bb9553

add documentation

a8be969

fix checks

9a16c1c

make sure keyring can be created durring testing

ae8e35b

Bart added 4 commits October 15, 2024 09:57

fix #4

41630fe

Fill out readme

013c5a9

update vignette

d992576

Merge branch 'readme' into get_pvol

8129cdd

style code and make time flexible (#2)

e23b881

PietrH self-requested a review October 18, 2024 07:06

Bart and others added 4 commits October 24, 2024 16:09

Add NEWS.md

3c4b113

devtools::document()

c8df378

usethis::use_tidy_description()

d55ce91

puts fields in standard order and alphabetises dependencies.

PietrH added 2 commits October 28, 2024 13:57

Change spacing for style, space between anonymous function init.

34054eb

limit to 80 cols

acca1b9

PietrH added 4 commits October 28, 2024 14:15

Add depends on R 4.1: use of base pipes and new syntax anonymous func…

aa5a85e

…tions

add spacing for readability

39f2d6c

Use = for function argument assignment

92c82e8

Avoid assignment in function calls! https://style.tidyverse.org/syntax.html#assignment

Use specific expectation where possible

4ed06fa

See also rlang::is_list(), avoids some inconsistency with S3/S4

PietrH and others added 8 commits October 28, 2024 16:17

use expect_type: easier to read

1b8d9bd

declare radars before expectations

b58ed7d

I prefer doing it in this order because it allows me to run the expectations in any order, it also allows us to change/remove earlier expectations without having to make changes to later expectations

assign timestamp before expectation, use lubridate to have tzone by d…

bceeb14

…efault before R4.3

define radars and timestamps before expectations, use expect_type ins…

710e6c7

…tead of expect_true(is.list : easier to read, run expectations out of order

lintr vignette

0469285

lint secrets file

50340d9

some more linting

d52985c

reduce lint remarks and avoid once false warning for lint (use nolint)

8009233

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `get_pvol` #1

add `get_pvol` #1

bart1 commented Oct 14, 2024

bart1 commented Oct 14, 2024

peterdesmet commented Oct 14, 2024

codecov bot commented Oct 14, 2024

bart1 commented Oct 14, 2024

bart1 commented Oct 15, 2024

bart1 commented Oct 16, 2024 •

edited

Loading

PietrH commented Oct 28, 2024

PietrH commented Oct 28, 2024 •

edited

Loading

bart1 commented Oct 28, 2024

PietrH commented Oct 28, 2024

bart1 commented Oct 28, 2024

PietrH commented Oct 28, 2024

bart1 commented Oct 28, 2024

PietrH commented Oct 28, 2024 •

edited

Loading

add get_pvol #1

Are you sure you want to change the base?

add get_pvol #1

Conversation

bart1 commented Oct 14, 2024

bart1 commented Oct 14, 2024

peterdesmet commented Oct 14, 2024

codecov bot commented Oct 14, 2024

Welcome to Codecov 🎉

bart1 commented Oct 14, 2024

bart1 commented Oct 15, 2024

bart1 commented Oct 16, 2024 • edited Loading

PietrH commented Oct 28, 2024

PietrH commented Oct 28, 2024 • edited Loading

bart1 commented Oct 28, 2024

PietrH commented Oct 28, 2024

bart1 commented Oct 28, 2024

PietrH commented Oct 28, 2024

bart1 commented Oct 28, 2024

PietrH commented Oct 28, 2024 • edited Loading

add `get_pvol` #1

add `get_pvol` #1

bart1 commented Oct 16, 2024 •

edited

Loading

PietrH commented Oct 28, 2024 •

edited

Loading

PietrH commented Oct 28, 2024 •

edited

Loading