Recycle all POSIXlt fields when doing period month/year addition

[DavisVaughan]

Part of https://github.com/tidyverse/lubridate/issues/1069, specifically my comment in https://github.com/tidyverse/lubridate/issues/1069#issuecomment-1268892740

So now we get this in the current state of R-devel again

as.Date("2019-01-30") + months(0:3)
#> [1] "2019-01-30" NA           "2019-03-30" "2019-04-30"

I've checked that this fixes slider too.

I'm convinced that this was an R-devel bug and I've sent them a message here about it https://stat.ethz.ch/pipermail/r-devel/2022-October/082066.html. Nevertheless, I feel like we could be more robust about recycling the fields of the POSIXlt objects where possible to avoid these issues altogether. Relying on base R's hidden recycling of the POSIXlt fields seems dangerous, and I'm not sure it is really documented well.

We also actually use base R recycling rules (accidentally?) here due to the + call. I've opened https://github.com/tidyverse/lubridate/issues/1070 to track a move towards tidyverse recycling rules, but for now I have tried to keep the base R recycling rules intact.

This patch doesn't actually fix the floor_date() issue in #1069. But it is possible the fixing of the r-devel bug itself will fix that issue. I am not sure yet, but will look at that in another PR. It probably implies we are partially recycling POSIXlt objects somewhere else, so it would be good to track it down either way.

[DavisVaughan]

oldrel-4 is failing because of timeDate: Needs R >= 3.6.0 which just happened on Sept 30

[vspinu]

The link to the report does not work for me. An alternative https://hypatia.math.ethz.ch/pipermail/r-devel/2022-October/082080.html

The patch looks good to me but I would rather see this fixed in R-devel instead. Adding so much code to fix a broken NA assignment doesn't seem right to me. Many others are likely affected, so should be fixed upstream. What do you think?

[DavisVaughan]

It was actually two problems in r-devel.

The PR was motivated by this one (not sure how I messed up the link before): https://stat.ethz.ch/pipermail/r-devel/2022-October/082075.html

And while working on it I found the other one you pointed to: https://stat.ethz.ch/pipermail/r-devel/2022-October/082080.html

We seem to rely on POSIXlt partial recycling or NA propagation in two places (that I am aware of). Here, and in floor_date() (which is what actually caused #1069) https://github.com/tidyverse/lubridate/blob/be024010ededa0d09cffe6e323777e3ab266f419/R/round.r#L381-L395

Ideally I think we wouldn't rely on this at all. It technically isn't a documented feature of POSIXlt, so I'd like to try and remove our dependence on it.

R core is definitely going to fix the first one, Martin said it was a bug. Fixing that first bug would fix the problems with floor_date() and +, but again I'm still not sure we should rely on this as it isn't fully thought out by R.

[vspinu]

@DavisVaughan I am still not sure about this PR. Judging from the threads there is a considerable effort from Martin to address the issues. And it looks like they have been already addressed in devel. No?

If we were to deal with this on our side then it would require adding an internal helper (normalize_POSIXlt or something, or is it the same as balancePOSIXlt that Martin already implemented in devel?) and then hunting all the occurrences of implicit recycling in our code in order to recycle. Thus this PR is somewhat half way through in that regard.

Shall we merge all the tests that you have created and then keep thinking of how to proceed further?

[vspinu]

I am a bit confused. I thought this is only R devel issues, but I see the same in R4.1 as well. So we do need to recycle on our side indeed :(.

[vspinu]

@DavisVaughan I have merged this except the last commit (given that it is already fixed in devel). Thanks!