Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 5, 2025

The timeFromExcelTime function had an off-by-one error when converting Excel dates 1-59 (January 1900 through February 1900) to Go time values. This caused dates to be returned one day earlier than expected.

Problem

When reading Excel date 2.0 (which should represent January 2, 1900), the function incorrectly returned January 1, 1900:

result := timeFromExcelTime(2.0, false)
// Expected: 1900-01-02
// Actual:   1900-01-01 ❌

Root Cause

The issue was in the Julian date conversion logic used for Excel dates ≤ 61. The function uses different conversion methods:

  • Julian conversion: for days 0-61 (handles Excel's leap year bug around Feb 29, 1900)
  • Gregorian conversion: for days > 61

The Julian conversion was working correctly for:

  • Day 0 (Excel epoch: 1899-12-30) ✅
  • Days 60-61 (around Excel's leap year bug) ✅

But days 1-59 had an incorrect offset, causing them to be off by one day.

Solution

Applied a targeted fix that adds +1.0 to the Julian date offset specifically for Excel days 1-59:

// Fix for off-by-one error in days 1-59
offset := OFFSET1900
if wholeDaysPart >= 1 && wholeDaysPart <= 59 {
    offset = OFFSET1900 + 1.0
}
date = julianDateToGregorianTime(MJD0, excelTime+offset)

This preserves the correct behavior for days 0, 60, and 61 (which handle Excel's leap year bug correctly) while fixing the offset error for the affected range.

Verification

  • timeFromExcelTime(2.0, false) now correctly returns 1900-01-02
  • All existing tests continue to pass ✅
  • Added comprehensive test coverage (TestTimeFromExcelTime_Issue2192) for edge cases

Fixes #2192.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] When reading a 1900-01-02 date from excel, got 1090-01-01 Fix off-by-one error in timeFromExcelTime for Excel dates 1-59 Aug 5, 2025
Copilot finished work on behalf of xuri August 5, 2025 03:33
- Made special date format result consistent with Excel
- Update unit tests
@xuri xuri marked this pull request as ready for review August 5, 2025 10:34
@xuri xuri added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 5, 2025
@xuri xuri changed the title Fix off-by-one error in timeFromExcelTime for Excel dates 1-59 This closes #2192, made special date format result consistent with Excel Aug 5, 2025
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.24%. Comparing base (11dcc8f) to head (fbd2325).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2193   +/-   ##
=======================================
  Coverage   99.24%   99.24%           
=======================================
  Files          32       32           
  Lines       30528    30562   +34     
=======================================
+ Hits        30297    30331   +34     
  Misses        153      153           
  Partials       78       78           
Flag Coverage Δ
unittests 99.24% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

The changes in the date.go will cause incorrect cell value if weekdays, elapsed date, and times in number format code.

I made some changes based on this branch.

@xuri xuri merged commit 5d2ee53 into master Aug 5, 2025
15 checks passed
@xuri xuri deleted the copilot/fix-2192 branch August 6, 2025 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When reading a 1900-01-02 date from excel, got 1090-01-01
3 participants