Conversation
…eads Fixes TauricResearch#1372 Cause: the OHLCV cache tests stamped a file's mtime with pd.Timestamp.timestamp(), which reads a naive value as UTC, while _cache_is_fresh reads it back with pd.Timestamp.fromtimestamp(), which returns local time. The two conventions disagreed by the machine's UTC offset -- far larger than the 900s TTL in most zones -- so whether a case passed depended on where it ran. Positive offsets made the file look newer than it was (3 failures); negative offsets made it look older, tripping a different set. Production is unaffected: it compares a local `now` from pd.Timestamp.today() against a local `written`, so both sides shift together. This is test-only, as the report says. Fix: stamp via datetime.timestamp(), which reads a naive value as local and is therefore the exact inverse of pd.Timestamp.fromtimestamp. The conversion lives in one helper in conftest so the four call sites across the two files cannot drift, and a round-trip test pins the requirement directly rather than leaving it implicit in every os.utime call. Verified on Asia/Kolkata (UTC+5:30), where 3 of the 19 cases failed before and all pass after. Modelling the conversion across zones shows the skew was exactly the UTC offset before and is zero after: timezone before after UTC +0s +0s Europe/London -3600s +0s Asia/Tokyo -32400s +0s Asia/Kolkata -19800s +0s America/New_York +14400s +0s America/Los_Angeles +25200s +0s Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
|
Thanks @shivsin25. Fixed in 2340fe4 (#1372). |
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
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.
Fixes #1372
Cause: the OHLCV cache tests stamped a file's mtime with pd.Timestamp.timestamp(), which reads a naive value as UTC, while _cache_is_fresh reads it back with pd.Timestamp.fromtimestamp(), which returns local time. The two conventions disagreed by the machine's UTC offset -- far larger than the 900s TTL in most zones -- so whether a case passed depended on where it ran. Positive offsets made the file look newer than it was (3 failures); negative offsets made it look older, tripping a different set.
Production is unaffected: it compares a local
nowfrom pd.Timestamp.today() against a localwritten, so both sides shift together. This is test-only, as the report says.Fix: stamp via datetime.timestamp(), which reads a naive value as local and is therefore the exact inverse of pd.Timestamp.fromtimestamp. The conversion lives in one helper in conftest so the four call sites across the two files cannot drift, and a round-trip test pins the requirement directly rather than leaving it implicit in every os.utime call.
Verified on Asia/Kolkata (UTC+5:30), where 3 of the 19 cases failed before and all pass after. Modelling the conversion across zones shows the skew was exactly the UTC offset before and is zero after: