Fix precisedelta rounding overflow across suppressed units

src/humanize/time.py

@@ -620,26 +620,51 @@ def precisedelta(
     # Due to rounding, it could be that a unit is high enough to be promoted to a higher
     # unit. Example: 59.9 minutes was rounded to 60 minutes, and thus it should become 0
     # minutes and one hour more.
-    if msecs >= 1_000 and SECONDS not in suppress_set:
-        msecs -= 1_000
-        secs += 1
-    if secs >= 60 and MINUTES not in suppress_set:
-        secs -= 60
-        minutes += 1
-    if minutes >= 60 and HOURS not in suppress_set:
-        minutes -= 60
-        hours += 1
-    if hours >= 24 and DAYS not in suppress_set:
-        hours -= 24
-        days += 1
-    # When adjusting we should not deal anymore with fractional days as all rounding has
-    # been already made. We promote 31 days to an extra month.
-    if days >= 31 and MONTHS not in suppress_set:
-        days -= 31
-        months += 1
-    if months >= 12 and YEARS not in suppress_set:
-        months -= 12
-        years += 1
+    #
+    # If the unit that the overflow would normally carry into has itself been
+    # suppressed, the overflow cannot stop there: it keeps carrying, compounding the
+    # conversion factor along the way, until it reaches a unit that has not been
+    # suppressed. Otherwise a rounding artifact can end up displayed as, say, "60000
+    # milliseconds" instead of being folded into "1 minute".
+    overflow_units = [MILLISECONDS, SECONDS, MINUTES, HOURS, DAYS, MONTHS, YEARS]
+    overflow_factors = [1_000, 60, 60, 24, 31, 12]
+    overflow_values = {
+        MILLISECONDS: msecs,
+        SECONDS: secs,
+        MINUTES: minutes,
+        HOURS: hours,
+        DAYS: days,
+        MONTHS: months,
+        YEARS: years,
+    }
+
+    for index, unit in enumerate(overflow_units[:-1]):
+        compound_factor = overflow_factors[index]
+        target_index = index + 1
+        while (
+            target_index < len(overflow_units) - 1
+            and overflow_units[target_index] in suppress_set
+        ):
+            compound_factor *= overflow_factors[target_index]
+            target_index += 1
+
+        target_unit = overflow_units[target_index]
+        if target_unit in suppress_set:
+            # Every unit above has been suppressed too; there is nothing left to
+            # carry the overflow into.
+            continue
+
+        if overflow_values[unit] >= compound_factor:
+            overflow_values[unit] -= compound_factor
+            overflow_values[target_unit] += 1
+
+    msecs = overflow_values[MILLISECONDS]
+    secs = overflow_values[SECONDS]
+    minutes = overflow_values[MINUTES]
+    hours = overflow_values[HOURS]
+    days = overflow_values[DAYS]
+    months = overflow_values[MONTHS]
+    years = overflow_values[YEARS]
 
     fmts = [
         ("%d year", "%d years", years),

tests/test_time.py

@@ -839,6 +839,60 @@ def test_precisedelta_suppress_units(
     )
 
 
+@pytest.mark.parametrize(
+    "val, min_unit, suppress, fmt, expected",
+    [
+        # Rounding the milliseconds up to a whole second (999937 microseconds
+        # rounds to 1000 milliseconds with "%0.0f") must still promote into
+        # minutes even though "seconds" is suppressed and can't absorb the
+        # carry itself.
+        (
+            dt.timedelta(seconds=59, microseconds=999937),
+            "milliseconds",
+            ["seconds"],
+            "%0.0f",
+            "1 minute",
+        ),
+        # Same overflow, but reached without any suppression, as a sanity
+        # check that both paths agree.
+        (
+            dt.timedelta(seconds=59, microseconds=999937),
+            "milliseconds",
+            [],
+            "%0.0f",
+            "1 minute",
+        ),
+        # The carry has to compound across two suppressed units (seconds and
+        # minutes) to reach hours.
+        (
+            dt.timedelta(seconds=3599, microseconds=999900),
+            "milliseconds",
+            ["seconds", "minutes"],
+            "%0.0f",
+            "1 hour",
+        ),
+        # A large value that legitimately belongs in "hours" because "days"
+        # is suppressed must *not* be carried into days: 308 hours is well
+        # under the compounded day/month threshold and is exactly what
+        # suppressing "days" is meant to produce.
+        (
+            dt.timedelta(days=73, seconds=72927, microseconds=928728),
+            "hours",
+            ["days"],
+            "%d",
+            "2 months and 308 hours",
+        ),
+    ],
+)
+def test_precisedelta_suppress_units_rounding_overflow(
+    val: dt.timedelta, min_unit: str, suppress: list[str], fmt: str, expected: str
+) -> None:
+    assert (
+        humanize.precisedelta(val, minimum_unit=min_unit, suppress=suppress, format=fmt)
+        == expected
+    )
+
+
 def test_precisedelta_bogus_call() -> None:
     assert humanize.precisedelta(None) == "None"