Skip to content

This closes #2372, fix precision loss for time.Duration cell values - #2373

Merged
xuri merged 2 commits into
qax-os:masterfrom
alliasgher:fix-duration-precision
Aug 9, 2026
Merged

This closes #2372, fix precision loss for time.Duration cell values#2373
xuri merged 2 commits into
qax-os:masterfrom
alliasgher:fix-duration-precision

Conversation

@alliasgher

Copy link
Copy Markdown
Contributor

Description

setCellDuration formats the Excel serial number with bitSize 32:

v = strconv.FormatFloat(value.Seconds()/86400, 'f', -1, 32)

value.Seconds() returns a float64 and the division is done in float64, but bitSize 32 tells FormatFloat to emit the shortest decimal that round-trips as a float32, so the extra precision is thrown away on write.

It is the only float write path in the package that does this — cell.go:278, cell.go:603, cell.go:646/648, lib.go:815, numfmt.go:5290/5544/5676, stream.go:358/722 and the calc.go sites all use 64. (cell.go:420 takes a caller-supplied bitSize from SetCellFloat, which is by design.)

Reproduced on master before the change:

duration stored with bitSize 64
100*24*time.Hour + 123456789ns 100 100.00000142889802
1m30s 0.0010416667 0.0010416666666666667

For the first one the entire fractional part is gone. Durations whose serial is exactly representable in a few decimal digits (25h30m, 3h15m45s) were already unaffected.

Related Issue

This closes #2372

Motivation and Context

Excel stores serials as IEEE doubles, so writing the full float64 expansion is what the format expects. Note this does change the raw value written into the XML for durations that are not exactly representable — that is the point of the fix, but it is an output change worth stating.

StreamWriter.SetRow (stream.go) also goes through setCellDuration, so it is fixed by the same one-line change.

How Has This Been Tested

The existing duration table in cell_test.go only asserts formatted output ("21:51:44" and friends), which passes both before and after — those are whole-second values that the number format rounds anyway, so they could not guard this. Added a raw-value assertion using Options{RawCellValue: true} for two durations that are not exactly representable; it fails on master with expected: "100.00000142889802" / actual: "100".

go test -race -timeout 60m ./... passes in full (839s — TestZip64 needs the longer timeout), and gofmt -s -l is clean on both changed files.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • I have added tests to cover my changes.

…lues

setCellDuration formatted the Excel serial number with bitSize 32, so
FormatFloat emitted the shortest decimal that round-trips as a float32 and
discarded the float64 precision the division produced.

Every other float write path in the package already uses 64.

Signed-off-by: alliasgher <alliasgher123@gmail.com>
@xuri xuri added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 9, 2026
@xuri xuri added this to v2.11.1 Aug 9, 2026
@xuri xuri moved this to Bugfix in v2.11.1 Aug 9, 2026
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.62%. Comparing base (a54c578) to head (1f28f3d).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2373   +/-   ##
=======================================
  Coverage   99.62%   99.62%           
=======================================
  Files          32       32           
  Lines       26994    26994           
=======================================
  Hits        26892    26892           
  Misses         53       53           
  Partials       49       49           
Flag Coverage Δ
unittests 99.62% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xuri xuri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! I've made some modification based on your branch.

@xuri
xuri merged commit d552a7e into qax-os:master Aug 9, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

Status: Bugfix

Development

Successfully merging this pull request may close these issues.

time.Duration loses precision due to 32-bit float formatting

2 participants