Skip to content

Fix negative timedelta parsing to match timedelta.__str__ semantics (fixes #23) - #41

Open
Yuren0312 wants to merge 1 commit into
wroberts:masterfrom
Yuren0312:fix-negative-timedelta-parsing
Open

Yuren0312 wants to merge 1 commit into
wroberts:masterfrom
Yuren0312:fix-negative-timedelta-parsing

Conversation

@Yuren0312

Copy link
Copy Markdown

Problem

timeparse() mis-parses the output of datetime.timedelta.__str__ for negative durations, causing a round-trip drift (issue #23).

timedelta.__str__ renders negative durations as -N day[s], HH:MM:SS[.ffffff], where the leading - negates only the day count and the clock part is always non-negative:

>>> str(timedelta(seconds=-86395))
'-1 day, 0:00:05'   # -86400 + 5 = -86395

timeparse instead extracted the leading sign and negated the entire field sum, so -1 day, 0:00:05 became -(86400 + 5) = -86405. Repeating the round-trip drifts every iteration:

t = timedelta(days=-1, seconds=5)
for _ in range(3):
    t = timedelta(seconds=timeparse(str(t)))
# -86405 -> -259195 -> -259205 -> ...   (keeps drifting)

Fix

  • Extracted the second-count computation into a small _sum_fields(mdict, sign) helper (behaviour-preserving refactor).
  • For the day+clock form (pattern 2: weeks/days followed by an HH:MM:SS clock), a leading minus now negates only the week/day portion while the clock stays positive, matching timedelta semantics. The DAYCLOCK (DD-HH:MM:SS) and unit forms keep their existing whole-expression sign behaviour.

Now -1 day, 0:00:05 → -86395 and the round-trip is stable.

Tests

  • Updated test_timeparse_11 / test_timeparse_12, whose old expected values encoded the previous (incorrect) whole-expression negation.
  • Added test_negative_day_clock and test_negative_timedelta_roundtrip covering str(timedelta(...)) round-trips.

Verification: python -m unittest discover -s pytimeparse/tests → 62 tests, OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant