⚡ Sangsakala Proxy 🏠 Home
Skip to content

Pack datetime subclasses as timestamps unless strict_types is set - #755

Closed
mkantautas wants to merge 1 commit into
msgpack:mainfrom
mkantautas:fix/datetime-subclass-timestamp
Closed

mkantautas wants to merge 1 commit into
msgpack:mainfrom
mkantautas:fix/datetime-subclass-timestamp

Conversation

@mkantautas

Copy link
Copy Markdown

With datetime=True and the default strict_types=False, the C Packer rejects a subclass of datetime.datetime, while the Python fallback packs it as a Timestamp. When a default callback is supplied, the C Packer calls it instead. This makes serialization depend on which backend is loaded.

The datetime checks now follow strict_types, as the other type checks do. Aware subclasses pack as timestamps, and naive subclasses raise the same ValueError as the fallback. Valid timedelta subclasses returned by subtraction are accepted. strict_types=True and datetime=False retain the existing subclass handling. Applications that intentionally serialize datetime subclasses through default can use strict_types=True to preserve that behavior.

For example, both backends now pack this value as d7ff64361d20661e3a1d:

import datetime
import msgpack

class DT(datetime.datetime):
    pass

value = DT(2024, 4, 16, 8, 43, 9, 420317, tzinfo=datetime.timezone.utc)
msgpack.packb(value, datetime=True)

The regressions cover timestamp bytes and round trips, callback dispatch, naive values, strict and disabled-datetime controls, and subtraction returning a timedelta subclass. A C-only test also checks that an invalid subtraction result raises TypeError; fallback behavior for invalid overrides is unchanged.

Validated on CPython 3.12.3 / Linux x86_64 with Cython 3.3.0:

  • Tests alone on the pinned base: three expected C-extension failures; fallback passes with the C-only test skipped.
  • Relaxing only the datetime checks still rejects a valid timedelta subclass; the complete patch passes all new cases.
  • Full C-extension suite: 151 passed, 1 skipped.
  • Full fallback suite: 147 passed, 5 skipped.
  • Ruff 0.17.2 check and format produced empty diffs; source and wheel builds passed.

Other Python versions, operating systems, PyPy and free-threaded builds were not run locally.

Refs #683, finding 16.

@mkantautas
mkantautas marked this pull request as ready for review October 26, 2026 07:57
@methane
methane requested a balanced review from Copilot November 4, 2026 10:13

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.

🟢 Approval recommended

No active review issues remain for the datetime subclass serialization changes.

No open issues 🎉

What changed in this PR

Aligns C-extension datetime subclass serialization with the Python fallback, addressing the backend inconsistency referenced in #683.

Changes:

  • Packs aware datetime subclasses as timestamps when datetime=True and strict_types=False.
  • Accepts timedelta subclass subtraction results and raises TypeError for invalid results.
  • Adds regression tests for timestamp round trips, callback dispatch, naive values, and strict or disabled-datetime behavior.
File Description
test/​test_timestamp.py Adds datetime subclass regression tests and a C-only invalid-subtraction check.
msgpack/​_packer.pyx Makes datetime checks respect strict_types and accepts timedelta subclasses.

Review effort: Balanced

@methane

methane commented Nov 4, 2026

Copy link
Copy Markdown
Member

Could you explain what motivated you to create this PR? If you have a real-world use case where this change is needed, please share it.

@mkantautas

Copy link
Copy Markdown
Author

There is no real-world use case here. Just wanted to get a free 200 claude plan with 100 PRs merged(already have over 100PRs merged).

https://claude.com/contact-sales/claude-for-oss

@mkantautas mkantautas closed this Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants