support hms from breaks_width

[bhogan-mitre]

Much thanks for the new breaks_width function in response to #190. Would there be any objection to supporting its use for hms as well?

library(hms)
library(ggplot2)

df <- data.frame(hours = 1:3, hms = hms(hours = 1:3))

ggplot(df, aes(x = hours, y = hms)) + 
  scale_y_time(breaks = scales::breaks_width(900))
#> Error in UseMethod("fullseq"): no applicable method for 'fullseq' applied to an object of class "c('hms', 'difftime')"

I made a simple change in a fork to add fullseq.hms and it appears to work fine.
bhogan-mitre@70a55db I'm happy to submit as a PR if you're open to it.

Looking ahead, it would be nice to have more intuitive specification from ggplot2::scale_*_time similar to date_breaks. Although I figured I'd start here. Thanks for considering.

[bhogan-mitre]

Thinking about this a bit more, I am less satisfied with the quick patch linked above. If any changes are made, I think it would be better to properly support intuitive units such as breaks_width("15 minutes") for hms variables.

[hadley]

Maybe breaks_width() needs to use a generic so there could be more control for different variable types?

[bhogan-mitre]

That sounds reasonable. Would that work similar to fullseq(), just up a level in breaks_width() as well?

So breaks_width.numeric() calls fullseq.numeric(), etc.

[bhogan-mitre]

I gave this another try via fullseq.hms and got it to handle input like breaks_width("15 min") by leveraging some of the logic that is used for POSIXct.

My issue at the moment is that it returns difftime instead of hms due to this namespace issue with suggested packages:

Namespace directives must refer to available functions, so they can not reference suggested packages. It’s possible to use package hooks and code to add this at run-time, but this is sufficiently complicated that I currently wouldn’t recommend it. Instead, you’ll have to design your package dependencies in a way that avoids this scenario.

via R-CMD-check on my fork

Could you advise how best to handle this @hadley?

[bhogan-mitre]

Maybe it would be best to handle this via a fullseq.difftime() function rather than fullseq.hms().

[bhogan-mitre]

I switched back to fullseq.difftime() and this appears to be working as intended. PR opened.