Conversation
Use an explicit invariant-culture format ("yyyy-MM-dd HH:mm:ss") for
DateTime keys that have a non-zero time component, instead of relying on
key.ToString() which is locale-sensitive. On Windows with an AM/PM locale
this was producing "9:30:00 AM" instead of the 24-hour form "09:30:00",
causing the CI test to fail.
The date-only path (TimeOfDay = 00:00:00) retains the existing short-date
CurrentCulture format because that path never contained the assertion that
was failing.
Updated the test to verify the full invariant format and explicitly assert
the absence of AM/PM suffixes.
Co-authored-by: Copilot <[email protected]>
This was referenced Mar 17, 2026
Closed
github-actions Bot
added a commit
that referenced
this pull request
Mar 18, 2026
The test 'series with datetime keys preserves time in Format' was failing on Windows because DateTime.ToString() uses the locale's AM/PM format. Apply the same fix as PR #643: use 'yyyy-MM-dd HH:mm:ss' with InvariantCulture for DateTime keys that have a non-zero time component. Co-authored-by: Copilot <[email protected]>
dsyme
approved these changes
Mar 18, 2026
dsyme
deleted the
repo-assist/fix-issue-639-datetime-locale-ci-49f926ecc1aae8cd
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Repo Assist — automated bug fix.
Summary
Fixes the Windows CI failure reported in #639, where the test
series with datetime keys preserves time in Formatwas failing becauseformatKeyusedkey.ToString()forDateTimevalues with a time component. On Windows with an AM/PM locale this produced"9:30:00 AM"instead of the expected"09:30:00".Root Cause
Formatting.formatKeyhad two paths:DateTimewithTimeOfDay = 00:00:00→ formatted with the current culture's short date format ("d")key.ToString()— locale-sensitivePath 2 is used for
DateTimekeys with a non-zero time component. On Windows with a culture that uses AM/PM (e.g.en-US),key.ToString()produces"1/1/2023 9:30:00 AM", which does not contain the"09:30:00"substring the test was asserting.Fix
Added an explicit case for
DateTimevalues with a non-zero time component that formats them as"yyyy-MM-dd HH:mm:ss"usingCultureInfo.InvariantCulture:This gives a consistent, unambiguous, zero-padded 24-hour format on all platforms and locales.
Trade-offs
Test Status
✅ All 612 tests pass on Linux (
dotnet test tests/Deedle.Tests/Deedle.Tests.fsproj -c Release).The updated test also asserts
"AM"and"PM"are absent from the output.Closes #639