◐ Shell
reader mode source ↗
Skip to content

Fix Timestamp.from_datetime returning wrong value for pre-epoch datetimes#662

Merged
methane merged 2 commits into
msgpack:mainfrom
bysiber:fix/timestamp-from-datetime-pre-epoch
Jun 2, 2026
Merged

Fix Timestamp.from_datetime returning wrong value for pre-epoch datetimes#662
methane merged 2 commits into
msgpack:mainfrom
bysiber:fix/timestamp-from-datetime-pre-epoch

Conversation

@bysiber

@bysiber bysiber commented Feb 20, 2026

Copy link
Copy Markdown
Contributor

Timestamp.from_datetime() uses int(dt.timestamp()) to compute the seconds component, but int() truncates towards zero. For pre-epoch datetimes with non-zero microseconds, this produces the wrong value:

import datetime
dt = datetime.datetime(1969, 12, 31, 23, 59, 59, 500000, tzinfo=datetime.timezone.utc)
# dt.timestamp() == -0.5
# int(-0.5) == 0  (truncation towards zero)
# Expected: Timestamp(seconds=-1, nanoseconds=500000000)
# Actual:   Timestamp(seconds=0, nanoseconds=500000000)  <-- +0.5s instead of -0.5s!

The sign of the time gets flipped. from_unix() already handles this correctly using floor division (int(unix_sec // 1)). This change makes from_datetime() consistent.

@bysiber
Fix Timestamp.from_datetime returning wrong value for pre-epoch times …
4ea42eb 
int() truncates towards zero, so for pre-epoch datetimes with non-zero
microseconds the seconds component gets the wrong value. For example,
datetime(1969, 12, 31, 23, 59, 59, 500000, UTC) has timestamp -0.5,
but int(-0.5) == 0, producing Timestamp(0, 500000000) (+0.5s after
epoch) instead of Timestamp(-1, 500000000) (-0.5s before epoch).

Use floor division (// 1) instead, consistent with from_unix().
@methane

methane commented Feb 24, 2026

Copy link
Copy Markdown
Member

@bysiber would you add a testcase for the fix to test_timestamp.py?

@methane methane requested a review from Copilot April 21, 2026 09:57
Copilot AI reviewed Apr 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Pull request overview

Fixes incorrect Timestamp.from_datetime() results for pre-epoch datetimes with fractional seconds by ensuring the seconds component is computed via flooring (consistent with Timestamp.from_unix()), avoiding int()’s truncation-toward-zero behavior.

Changes:

  • Update Timestamp.from_datetime() to compute seconds using int(dt.timestamp() // 1) instead of int(dt.timestamp()).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Hide details View details @methane methane merged commit 98d2c79 into msgpack:main Jun 2, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methane methane methane approved these changes

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bysiber @methane