Skip to content

Add millisecond units to time expressions - #40

Open
GreedyC wants to merge 2 commits into
wroberts:masterfrom
GreedyC:codex/millisecond-units
Open

GreedyC wants to merge 2 commits into
wroberts:masterfrom
GreedyC:codex/millisecond-units

Conversation

@GreedyC

@GreedyC GreedyC commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #22 (also relates to #17).

Add millisecond units (ms, msec(s), millis, and millisecond(s)) to the existing unit-based parser. They can be used alone or after larger units, such as 1s250ms and 1 minute, 500 milliseconds. Whole-second totals still return integers; fractional totals return floats.

This leaves the existing calculation path for expressions without milliseconds unchanged, keeping the fractional-unit fix in #38 separate. Clock-format changes are out of scope.

Validation

  • Baseline: 57 tests passed. Before the implementation, the new positive millisecond tests failed on the missing support.
  • Full unittest discovery: 61 tests passed on Python 3.11, 3.13, and 3.14 (macOS).
  • Module doctests and git diff --check passed.
  • Tests cover aliases, signs, mixed units, fractional milliseconds, integer return types, and invalid input/separators.

Codex assisted with the implementation, tests, and this description. No human review or hosted CI result is claimed; Copilot review was not available locally.

@wroberts

Copy link
Copy Markdown
Owner

Thanks for this. Millisecond support (#22) is a useful addition, and the regex is nicely restricted: millis only after a seconds field, with an invalid-input test for each edge case.

1. Float drift in the new branch (please fix before merge)

Summing with 1e-3 * float(v) brings back the same class of bug that #38 just fixed for fractional units:

>>> parse('9ms')
0.009000000000000001
>>> parse('4.1m500ms')
246.49999999999997
>>> parse('2.3d1ms')
198720.00099999996

In a sweep of 0ms-20000ms, 2,674 values didn't equal ms/1000. The tests don't catch this because they use assertAlmostEqual.

Suggested fix: sum the millis branch with Decimal (as #38 now does on the integer path), and convert to float only at the end. Then result == int(result) can decide between int and float without depending on float error, and 9ms would give exactly 0.009. Please also switch the new tests to assertEqual, so exactness is covered.

2. Duplicate code path

The new if mdict.get('millis') is not None: branch repeats the summing logic. With the Decimal change above, it may be simpler to fold millis into the existing integer/float branches.

3. Merge conflict

This now conflicts with master in testtimeparse.py, because #38 added a test at the same spot. Keeping both test blocks resolves it, and the suite passes (64 tests) after that.

@GreedyC

GreedyC commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the precise examples. Merged current master, retained both sets of regression tests, and folded milliseconds into the Decimal calculation path instead of keeping a separate float sum. The new millisecond assertions now use assertEqual, including all three drift examples. All 64 tests and the README/module doctests pass; a sweep from 0ms through 20000ms has zero mismatches against ms / 1000. Prepared with Codex assistance.

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.

Feature Request: Milliseconds Support

2 participants