| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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().
|
@bysiber would you add a testcase for the fix to test_timestamp.py? |
Sorry, something went wrong.
There was a problem hiding this comment.
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:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| :rtype: Timestamp | ||
| """ | ||
| return Timestamp(seconds=int(dt.timestamp()), nanoseconds=dt.microsecond * 1000) | ||
| return Timestamp(seconds=int(dt.timestamp() // 1), nanoseconds=dt.microsecond * 1000) |
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Sorry, something went wrong.
`Timestamp.from_datetime()` computes the whole-second part from the float `datetime.timestamp()`. A float64 cannot hold microsecond precision for datetimes far from the epoch, so `timestamp()` rounds the seconds up while the exact `microsecond` is still used for the nanoseconds. The result is a `Timestamp` one second in the future, and near `datetime.max` it raises `OverflowError`: ```python >>> import datetime as dt >>> from msgpack.ext import Timestamp >>> d = dt.datetime(3000, 1, 1, 0, 0, 0, 999999, tzinfo=dt.timezone.utc) >>> Timestamp.from_datetime(d).to_datetime() datetime.datetime(3000, 1, 1, 0, 0, 1, 999999, tzinfo=datetime.timezone.utc) # +1 second >>> Timestamp.from_datetime(dt.datetime(9999, 12, 31, 23, 59, 59, 999999, tzinfo=dt.timezone.utc)) OverflowError: date value out of range ``` The Cython packer already uses integer `timedelta` arithmetic and is correct. This makes the pure-Python `from_datetime()` do the same, so the two paths agree and the round-trip invariant `from_datetime(d).to_datetime() == d` holds for far-future datetimes. Naive datetimes keep their existing local-time interpretation (matching `datetime.timestamp()`). Follow-up to #662, which fixed the pre-epoch rounding direction in the same method. Existing tests are unchanged; I added a regression test covering the far-future round-trip, the exact seconds/nanoseconds, agreement with packing the datetime directly, and the former `OverflowError` case. Full suite is green on both the C-extension and pure-Python paths.
| Back | FazBrowse Home | New Git URL |
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:
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.