aphalo / photobiology

Package ‘photobiology’ defines a system of classes for storing spectral data and accompanying methods and operators. This is the core of a suite of R packages for photobiological calculations.
4 stars 1 forks source link

sunrise_time does not update time value for requested time zone #7

Closed putmanlab closed 2 years ago

putmanlab commented 2 years ago

It seems sunrise_time is not correct for non-UTC times with recent versions of lubridate or R, as shown in the User Guide.

library(lubridate)
library(photobiology)

my.geocode <- data.frame(lat = 60.16, lon = 24.93, address = "Helsinki")
my.datetime <- as_datetime("2022-07-10 12:00:00", tz="EET")

sunrise_time(my.datetime, geocode = my.geocode, tz = "UTC")
[1] "2022-07-10 01:13:46 UTC"

sunrise_time(my.datetime, geocode = my.geocode, tz = "EET")
[1] "2022-07-10 01:13:46 EEST"

> sessionInfo()
R version 4.2.0 (2022-04-22)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.4 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/openblas-pthread/libblas.so.3
LAPACK: /usr/lib/x86_64-linux-gnu/openblas-pthread/liblapack.so.3

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] photobiology_0.10.2 tibble_3.1.7        lubridate_1.8.0    

loaded via a namespace (and not attached):
 [1] compiler_4.2.0    pillar_1.7.0      prettyunits_1.1.1 remotes_2.4.2     tools_4.2.0       testthat_3.1.4    pkgbuild_1.3.1   
 [8] pkgload_1.2.4     memoise_2.0.1     lifecycle_1.0.1   pkgconfig_2.0.3   rlang_1.0.2       DBI_1.1.3         cli_3.3.0        
[15] fastmap_1.1.0     withr_2.5.0       dplyr_1.0.9       generics_0.1.2    desc_1.4.1        fs_1.5.2          vctrs_0.4.1      
[22] devtools_2.4.3    rprojroot_2.0.3   tidyselect_1.1.2  glue_1.6.2        R6_2.5.1          processx_3.6.1    fansi_1.0.3      
[29] sessioninfo_1.2.2 callr_3.7.0       purrr_0.3.4       magrittr_2.0.3    ps_1.7.1          ellipsis_0.3.2    usethis_2.1.6    
[36] assertthat_0.2.1  utf8_1.2.2        cachem_1.0.6      crayon_1.5.1      brio_1.1.3   

However, regressing to earlier versions of lubridate and R but using the same photobiology version results in the correct time.

library(lubridate)
library(photobiology)

my.geocode <- data.frame(lat = 60.16, lon = 24.93, address = "Helsinki")
my.datetime <- as_datetime("2022-07-10 12:00:00", tz="EET")

sunrise_time(my.datetime, geocode = my.geocode, tz = "UTC")
[1] "2022-07-10 01:13:46 UTC"

sunrise_time(my.datetime, geocode = my.geocode, tz = "EET")
[1] "2022-07-10 04:13:46 EEST"

sessionInfo()
R version 3.6.3 (2020-02-29)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Debian GNU/Linux 10 (buster)

Matrix products: default
BLAS/LAPACK: /usr/lib/x86_64-linux-gnu/libopenblasp-r0.3.5.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8       
 [4] LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8    LC_MESSAGES=C             
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C              
[10] LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] photobiology_0.10.2 tibble_3.0.1        lubridate_1.7.8    

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.4.6      compiler_3.6.3    pillar_1.4.3      prettyunits_1.1.1 remotes_2.1.1    
 [6] tools_3.6.3       testthat_2.3.2    digest_0.6.25     pkgbuild_1.0.6    pkgload_1.0.2    
[11] memoise_1.1.0     lifecycle_0.2.0   pkgconfig_2.0.3   rlang_0.4.5       cli_2.0.2        
[16] rstudioapi_0.11   withr_2.2.0       dplyr_0.8.5       generics_0.0.2    desc_1.2.0       
[21] fs_1.4.1          vctrs_0.2.4       devtools_2.3.0    rprojroot_1.3-2   tidyselect_1.0.0 
[26] glue_1.4.0        R6_2.4.1          processx_3.4.2    fansi_0.4.1       sessioninfo_1.1.1
[31] purrr_0.3.4       callr_3.4.3       magrittr_1.5      backports_1.1.6   ps_1.3.2         
[36] ellipsis_0.3.0    usethis_1.6.0     assertthat_0.2.1  crayon_1.3.4  
aphalo commented 2 years ago

Many thanks for this report! I will investigate as soon as possible what has changed in 'lubridate', and if the bug is in 'photobiology' or 'lubridate'. I suspect the problem may be caused by a work-around I have in 'photobiology' for a situation where the tz was not set correcly by 'lubridate'.

aphalo commented 2 years ago

I suspect the problem has to do with the change in 'lubridate' from "UTC" as default time zone to "" as default time zone in functions like "now()" and conversion functions like "ymd()".

aphalo commented 2 years ago

This guess was wrong. The problem stems at least in part from the use of lubridate::today() as default for date in sunrise_time(), sunset_time() and noon_time() in 'photobiology' as today() returns a Date instead of a POSIXct and thus lacks a time zone setting. Another problem is that lubridate::floor_date() converts POSIXct objects to Date, thus dropping the time zone information. I have started the revision to the code but it is not yet working when the argument passed to date has length > 1.

aphalo commented 2 years ago

Added unit tests and tested with 'lubridate' 1.7.8, 1.7.9.2 and 1.8.0. Tested with 'tibble' 3.1.0 and 3.1.7. All under R 4.2.1. The previously buggy functions now return the correct values in all cases, independently of input being a Date or POSIXct object. For Date arguments passed to date the default is tz = "UTC". The default for date is now lubridate function now() instead of today(), now called with tz = "UTC". Currently tz is used to determine to which data an instant in time belongs.

putmanlab commented 2 years ago

It is fixed for me. Thanks for the rapid response, and many thanks for developing a very useful package!

aphalo commented 2 years ago

You are welcome! Once again thanks for reporting the problem!