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

FR: as.hms.units() #9

Closed
krlmlr opened this issue Sep 13, 2016 · 6 comments
Closed

FR: as.hms.units() #9

krlmlr opened this issue Sep 13, 2016 · 6 comments

Comments

@krlmlr
Copy link
Contributor

krlmlr commented Sep 13, 2016

Simply as.hms.units <- function(x, ...) as.hms(as.dt(x, ...)), but probably needs to import hms.

@edzer edzer closed this as completed in 69bc1e7 Sep 14, 2016
@edzer
Copy link
Member

edzer commented Sep 16, 2016

I now realize that hms is used for time of day, not duration. The way you suggest to implement it simply coerces a difftime object (time duration) into a hms, but that means something different. I will remove this until we've sorted it out. udunits does have support for absolute time; we could coerce that to POSIXct, and then drop day and retain a hms; the other way around is impossible without knowing the day.

@edzer edzer reopened this Sep 16, 2016
@krlmlr
Copy link
Contributor Author

krlmlr commented Sep 16, 2016

hms wraps difftime, and so can be used for both: duration (as difference of two times) and time of day (like in Sys.time() - as.POSIXct(Sys.Date())).

@edzer
Copy link
Member

edzer commented Sep 16, 2016

I can see that, but that is not how the hms package explains what hms values mean. So: syntactically yes, semantically no, and measurement units are about meaning. Subclassing time-of-day from a time duration class is a conceptual mistake: is 12:00:00 twice as late as 06:00:00? hms thinks so:

> library(hms)
> hms(hours=6)
06:00:00
> hms(hours=6)*2
Time difference of 43200 secs

This should of course have triggered an error.

@krlmlr
Copy link
Contributor Author

krlmlr commented Sep 16, 2016

Thanks, I shall adapt the README then. The hms class doesn't specify preferred usage, it's just a container with nice formatting intended to be compatible to difftime.

See tidyverse/hms#18 for the conversion issue.

@edzer
Copy link
Member

edzer commented Sep 16, 2016

The better fix would be to make hms unambiguous (and not derive from difftime)

@edzer
Copy link
Member

edzer commented Dec 12, 2016

O.K., you clarified that the meaning of hms 09:00:00 can be one of

  • a duration of 9 hours (since 00:00 midnight)
  • 9 o'clock, as of "the shop opens at 9 o'clock in the morning", reflecting time-of-day

Since this cannot be be converted unambiguously in an SI unit of time duration (as package units understands them), I close this issue.

@edzer edzer closed this as completed Dec 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants