diff --git a/src/humanize/time.py b/src/humanize/time.py index 82a80424..f8c193eb 100644 --- a/src/humanize/time.py +++ b/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), diff --git a/tests/test_time.py b/tests/test_time.py index f63b5cb0..032b60a3 100644 --- a/tests/test_time.py +++ b/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"