Deprecations causing large amount of churn, and result in less elegant code

[hniksic]

TL;DR The recent deprecation changes are really causing a large amount of churn, and result in code that is harder to read.

I'm aware that the intention is to make panics more obvious and the potentially panicking code more explicit. I'm also aware that bringing this up might be beating a dead horse, as the discussions were most likely already had. But still, as someone who works on a large code base that heavily relies on chrono, I feel it'd be irresponsible not to bring this up.

Last year we had to make many changes to address chrono deprecations, most notably change every NaiveTime::from_hms() to from_hms_opt(...).unwrap(). This is especially jarring with constants, where implicit panic is exactly what you want. E.g. I see nothing wrong with NaiveTime::from_hms(3, 0, 0) and don't find the new expression NaiveTime::from_hms_opt(3, 0, 0).unwrap() an improvement, especially in a codebase that is otherwise careful to document its use of unwraps. Requiring that change feels akin to requiring to change every use of vec[index] to vec.get(index).unwrap() - while it does make the panic explicit, it also has a strong impact on readability.

In our business logic the result was hundreds of diffs like this one:

         static ref FIRST_OF_JULY_2021: DateTime<Utc>
-            = FixedOffset::east(3600).ymd(2021, 7, 1).and_hms(0, 0, 0).with_timezone(&Utc);
+            = FixedOffset::east_opt(3600).unwrap().with_ymd_and_hms(2021, 7, 1, 0, 0, 0).unwrap().with_timezone(&Utc);

And now a new chrono update is bringing new deprecations, this time on Duration constructors. We are now churning out these changes:

                 let prioritized = stats_for_day
                     .iter()
                     .filter(|s| {
-                        s.run_time.time() - Duration::minutes(10) < prioritized_time
-                            && s.run_time.time() + Duration::minutes(10) > prioritized_time
+                        s.run_time.time() - Duration::try_minutes(10).unwrap() < prioritized_time
+                            && s.run_time.time() + Duration::try_minutes(10).unwrap()
+                                > prioritized_time
                     })

or:

     pub fn get_hours(from_time: &DateTimeTz, to_time: &DateTimeTz) -> Vec<DateTimeTz> {
-        let mut from = from_time.duration_trunc(Duration::hours(1)).unwrap();
+        let mut from = from_time
+            .duration_trunc(Duration::try_hours(1).unwrap())
+            .unwrap();

It's not just a matter of work - we could easily avoid it with an appropriate #[allow] directive. The thing is, we want to use chrono the right way, and it seems that what chrono now considers right is just too strict. Durations constructed with Duration::hours(<constant>) are a very frequent atom in chrono usage. Having to use Duration::try_hours(<constant>).unwrap() makes it significantly less readable in chrono-heavy code, and has detrimental impact on the whole ecosystem. I would like to appeal to people who make the decisions to reconsider these deprecations.

[pitdicker]

You probably intended this issue for chrono and not chrono-tz? Just published 0.4.36 which undoes the deprecations on TimeDelta.

[hniksic]

You probably intended this issue for chrono and not chrono-tz?

Well, this is embarrassing. :) Yes, I did.

Just published 0.4.36 which undoes the deprecations on TimeDelta.

Oh, nice! Those are the ones on Duration::hours(), etc.?

[pitdicker]

Yes. It will need a new 0.4.37 unfortunately because of an accidental breaking change.