tidyverse / lubridate

Make working with dates in R just that little bit easier
https://lubridate.tidyverse.org
GNU General Public License v3.0
728 stars 207 forks source link

`as.period(<Interval>, unit = "month")` needs to recycle the `0` `year` #1109

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

Seen here https://github.com/tidyverse/dplyr/issues/6789

library(lubridate)

x <- interval(
  start = as.Date("2019-01-01") + 0:2,
  end = as.Date("2023-01-02") + c(0, 100, 200),
  tz = "UTC"
)
x
#> [1] 2019-01-01 UTC--2023-01-02 UTC 2019-01-02 UTC--2023-04-12 UTC
#> [3] 2019-01-03 UTC--2023-07-21 UTC

y <- as.period(x, "months")
y
#> [1] "48m 1d 0H 0M 0S"  "51m 10d 0H 0M 0S" "54m 18d 0H 0M 0S"

# look at `year` - oh no!
unclass(y)
#> [1] 0 0 0
#> attr(,"year")
#> [1] 0
#> attr(,"month")
#> [1] 48 51 54
#> attr(,"day")
#> [1]  1 10 18
#> attr(,"hour")
#> [1] 0 0 0
#> attr(,"minute")
#> [1] 0 0 0

# makes these inconsistent
year(y)
#> [1] 0
month(y)
#> [1] 48 51 54

A large amount of things rely on the Period internals being recycled to the same length, so I think this is a bug (unlike where POSIXlt allows partially recycled elements, which is just a mess)

Seems relatively easy to fix by going through the new() Period constructor, which does recycling and NA standardization for us.

DavisVaughan commented 1 year ago

Same problem with casting existing Period object to months (goes through a different code path, but looks like a copy/paste)

library(lubridate)
#> 
#> Attaching package: 'lubridate'
#> The following objects are masked from 'package:base':
#> 
#>     date, intersect, setdiff, union

x <- years(1:5)

y <- as.period(x, "month")

unclass(y)
#> [1] 0 0 0 0 0
#> attr(,"year")
#> [1] 0
#> attr(,"month")
#> [1] 12 24 36 48 60
#> attr(,"day")
#> [1] 0 0 0 0 0
#> attr(,"hour")
#> [1] 0 0 0 0 0
#> attr(,"minute")
#> [1] 0 0 0 0 0

Created on 2023-03-15 with reprex v2.0.2.9000