search

vspinu / timechange

Efficient Updating of Date-Times
GNU General Public License v3.0
30 stars 3 forks source link

New timechange update breaks recursive revdeps of lubridate #25

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

Hi @vspinu, I see you have updated timechange on CRAN.

Did you happen to run the revdeps of lubridate before you sent it in? It seems like the changes in timechange have broken tibbletime, a package of mine that uses lubridate.

We are happy to run the lubridate revdeps using our AWS cloud check system next time! It is massively parallel and very cheap, fast, and easy for us, so feel free to reach out.

Here are two of the differences:

library(lubridate)

x <- as.Date("2019-01-01")
# timechange 0.1.1
floor_date(x, "2 weeks")
#> Warning in .normalize_multi_week_unit(unit): Multi-week unit is not supported.
#> Ignoring.
#> [1] "2018-12-30"

# timechange 0.2.0
floor_date(x, "2 weeks")
#> Error in validate_rounding_nunit(.Call(C_parse_unit, as.character(unit))): Rounding with week > 1 is not supported. Use aseconds for arbitrary units.
# timechange 0.1.1
floor_date(x, "yearly")
#> [1] "2019-01-01"

# timechange 0.2.0
floor_date(x, "yearly")
#> Error in validate_rounding_nunit(.Call(C_parse_unit, as.character(unit))): Invalid unit specification 'yearly' (at ly)
vspinu commented 1 year ago

Hmm, I didn't run recursive revdeps indeed. I will do that next time.

Regarding the multi-week unit, it was never supported and I actually saw people using it, so I think it's a good thing to finally break it.

Regarding "yearly", it is never documented as a use case. I was not even aware of it. Do you think it is really worth supporting?

We are happy to run the lubridate revdeps using our AWS cloud check system next time! It is massively parallel and very cheap, fast, and easy for us, so feel free to reach out.

That would be great indeed. Would I be able to trigger it myself without pinging one of you?

DavisVaughan commented 1 year ago

I did a fairly quick search and it looks like I might be the only person doing yearly/monthly/daily etc, so I can just fix it on my end and we can leave it deprecated.

I totally understand about deprecating multi-week units. It would have been nice to have a little lead time on that though (as opposed to getting a surprise 2 week notice from cran).

Unfortunately cloud checks are currently internal to Posit so you will need to ping one of us. We hope to open source the infrastructure eventually.