tidyverse / hms

A simple class for storing time-of-day values
https://hms.tidyverse.org/
Other
138 stars 25 forks source link

as.hms fails with timezone? #32

Closed randomgambit closed 7 years ago

randomgambit commented 7 years ago

Hello everyone,

Thanks again for this nice package. Consider this

library(hms)
library(lubridate)
library(tidyverse)

options(digits.secs=3)

dataframe <- data_frame(gmt_time = c('2016-07-08 04:30:10.690'), value = c(1))

dataframe <- dataframe %>% 
  mutate(gmt_time = ymd_hms(gmt_time),
         est_time = with_tz(gmt_time, 'America/New_York'),
         myhour1 = as.hms(est_time),
         myhour2 = format(est_time, format='%H:%M:%OS'),
         myhour3 = as.hms(myhour2))

> dataframe
# A tibble: 1 × 6
                gmt_time value               est_time         myhour1      myhour2  myhour3
                  <dttm> <dbl>                 <dttm>          <time>        <chr>   <time>
1 2016-07-08 04:30:10.69     1 2016-07-08 00:30:10.69 04:30:10.690000 00:30:10.690 00:30:10

Why does hms returns the wrong time? It returns the GMT time instead of EST, although you can see that est_time is correctly converted.

Thanks for your help!

krlmlr commented 7 years ago

This looks like a duplicate of #28.

randomgambit commented 7 years ago

Ha! OK sorry about that. Well that gives you a practical example of how people use these packages! ;-)

krlmlr commented 7 years ago

Indeed. Does my proposed fix to lubridate work for you?

randomgambit commented 7 years ago

I think it makes totally sense.

People a more likely to know about lubridate than hms. If I were you, I would include all of the hms functions into lubridate directly so that you have a unique package that handles dates and times at the same time (ha ha).

krlmlr commented 7 years ago

Thanks. It would be helpful if you could comment at the lubridate PR. Originally, hms was conceived as a lightweight package to store time-of-day or duration data as a number of seconds, and print it nicely. The lubridate package has much better tools for handling this kind of data.

randomgambit commented 7 years ago

what do you mean comment at the lubridate PR? posting on the lubridate github?

I see your point of keeping hms as lightweight as possible, but consider the very frequent setting where someone works with intraday timestamped data, and needs to filter data between some business hours every day. Using hms(timestamp) > hms('09:30') is a natural way to do so. The horrible way to do so is to extract the time part of a timestamp (in string format) and compare to > '09:30:00'

Right now, lubridate does not have a good support for time of day and that is why I think both packages are complementary. Ultimately, they could be combined into one (in the same fashion as in Pandas) but this is a matter of taste.

krlmlr commented 7 years ago

Could you comment at https://github.com/hadley/lubridate/pull/538? You could also install this PR via devtools::install_github("hadley/lubridate#538"), or download/clone from https://github.com/krlmlr/lubridate/tree/f-hms.

krlmlr commented 7 years ago

Happy to discuss further improvements of time-of-day handling there.

github-actions[bot] commented 3 years ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.