Fix check_tz to return desired result when timezone is invalid

[ateucher]

A small thing I noticed:

When an invalid timezone is passed to the timezone_out argument in dbConnect(), the correct warning is thrown but the invalid timezone is kept due to a bug in check_tz (should be ""):

library(RPostgres) con <- postgresDefault(timezone = "+01:00") #> Warning: Invalid time zone '+01:00', falling back to local time. #> Set the `timezone` argument to a valid time zone. #> CCTZ: Unrecognized output timezone: "+01:00" con@timezone #> [1] "+01:00" dbDisconnect(con) RPostgres:::check_tz("+01:00") #> Warning: Invalid time zone '+01:00', falling back to local time. #> Set the `"+01:00"` argument to a valid time zone. #> CCTZ: Unrecognized output timezone: "+01:00" #> [1] "+01:00"

^{Created on 2021-01-15 by the reprex package (v0.3.0)}

This PR fixes this by assigningtimezone to the parent env with <<- within tryCatch in check_tz, and adds a couple of new tests.

[krlmlr]

Thanks, good catch. I think the intention was to have check_tz() return the repaired time zone, no need to assign to the parent environment. Would you like to tweak?

[ateucher]

Thanks @krlmlr. As submitted, check_tz() does return the repaired time zone... it's the error handler intryCatch() that assigns the repaired timezone to the check_tz() environment (which is then returned).

Unless I'm overthinking it, the other way to do it is to wrap lubridate::force_tz in lubrdate::tz(), and return the result of tryCatch() (see below):

check_tz <- function(timezone) { arg_name <- deparse(substitute(timezone)) tryCatch( lubridate::tz(lubridate::force_tz(as.POSIXct("2021-03-01 10:40"), timezone)), error = function(e) { warning( "Invalid time zone '", timezone, "', ", "falling back to local time.\n", "Set the `", arg_name, "` argument to a valid time zone.\n", conditionMessage(e), call. = FALSE ) "" } ) } check_tz("Etc/GMT+8") #> [1] "Etc/GMT+8" check_tz("not_a_timezone") #> Warning: Invalid time zone 'not_a_timezone', falling back to local time. #> Set the `"not_a_timezone"` argument to a valid time zone. #> CCTZ: Unrecognized output timezone: "not_a_timezone" #> [1] ""

^{Created on 2021-01-18 by the reprex package (v0.3.0)}

[krlmlr]

Thanks, I was too confused by <<-; changed it to a pattern similar to yours.

[ateucher]

Oh that is better - thanks!