Skip to content

feat(types): add datetime.date and datetime.datetime support for Tag values (ITL-45) - #188

Merged
eywalker merged 9 commits into
mainfrom
eywalker/itl-45-add-support-for-datetime-in-tag-values
Jun 29, 2026
Merged

feat(types): add datetime.date and datetime.datetime support for Tag values (ITL-45)#188
eywalker merged 9 commits into
mainfrom
eywalker/itl-45-add-support-for-datetime-in-tag-values

Conversation

@kurodo3

@kurodo3 kurodo3 Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bumps starfix to ~=0.4.0, which adds pa.timestamp hashing support — enabling content_hash() on Tags with datetime columns
  • Adds datetime.date ↔ pa.date32() bidirectional mapping in UniversalTypeConverter (fixes arrow_type_to_python_type(pa.date32()) returning Any)
  • Adds date and datetime to the TagValue and SupportedNativePythonData public type aliases (removes the TODO comment)
  • Adds 11 integration tests in TestTagDatetimeValues covering construction, round-trip, Arrow schema, content hashing, naive-datetime rejection, and schema inference for both types

Test plan

  • uv run pytest test-objective/unit/test_semantic_types.py -k "date" -v — 3 new type-converter tests pass
  • uv run pytest test-objective/unit/test_tag.py::TestTagDatetimeValues -v — all 11 integration tests pass
  • uv run pytest test-objective/ tests/ -q — full suite: 4341 passed, 56 skipped, 13 xfailed

Closes ITL-45

🤖 Generated with Claude Code

@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

Pull request overview

This PR formalizes datetime.date and datetime.datetime as supported Tag value types by aligning public type aliases with the Arrow conversion layer, and by upgrading starfix so content_hash() can hash Arrow timestamp columns.

Changes:

  • Bump starfix to ~=0.4.0 to enable hashing for pa.timestamp(...) columns used by datetime-valued Tags.
  • Extend UniversalTypeConverter with an explicit date ↔ pa.date32() mapping (and reverse mapping via pa.types.is_date).
  • Update public type aliases (TagValue, SupportedNativePythonData) and add integration/unit tests covering construction, round-trip, schema inference, and hashing for date/datetime Tag values.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pyproject.toml Bumps starfix to ~=0.4.0 to support timestamp hashing.
uv.lock Locks the resolved starfix==0.4.0 artifact metadata.
src/orcapod/semantic_types/universal_converter.py Adds direct date → pa.date32() mapping and pa.date* → date reverse mapping.
src/orcapod/types.py Expands public type aliases to include date and datetime.
test-objective/unit/test_semantic_types.py Adds converter tests for date type mapping and schema round-trip.
test-objective/unit/test_tag.py Adds TestTagDatetimeValues integration tests for Tag behavior with date/datetime values.
superpowers/specs/2026-06-29-itl-45-datetime-tag-values-design.md Adds design doc describing goals, scope, and the chosen Arrow mappings.
superpowers/plans/2026-06-29-itl-45-datetime-tag-values.md Adds an implementation plan for the change set and verification steps.

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

Comment thread test-objective/unit/test_tag.py Outdated
ctx = _make_context()
tag = Tag({"dob": date(2024, 1, 15)}, data_context=ctx)
table = tag.as_table()
assert pa.types.is_date(table.schema.field("dob").type)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Changed the assertion to assert table.schema.field("dob").type == pa.date32() — this pins the exact type and will catch any silent regression to date64.

Comment thread test-objective/unit/test_tag.py Outdated
dt = datetime(2024, 1, 15, 12, 0, 0, tzinfo=timezone.utc)
tag = Tag({"ts": dt}, data_context=ctx)
table = tag.as_table()
assert pa.types.is_timestamp(table.schema.field("ts").type)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Changed to assert table.schema.field("ts").type == pa.timestamp("us", tz="UTC") — this pins both the microsecond precision and the UTC timezone, fully encoding the converter contract.

Comment thread superpowers/specs/2026-06-29-itl-45-datetime-tag-values-design.md Outdated
Comment thread superpowers/plans/2026-06-29-itl-45-datetime-tag-values.md Outdated
- Assert exact `pa.date32()` type in `test_date_tag_as_table_schema` instead
  of the weaker `pa.types.is_date()` predicate, preventing silent regressions
  to date64
- Assert exact `pa.timestamp("us", tz="UTC")` type in
  `test_datetime_tag_as_table_schema` instead of `pa.types.is_timestamp()`,
  pinning both the unit and timezone as required by the converter contract
- Fix typo "accomodate" → "accommodate" in the spec and plan docs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kurodo3

kurodo3 Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Review round addressed (commit 0f3e2edc)

Four comments fixed in a single commit:

Comment Fix
test_date_tag_as_table_schema used pa.types.is_date() — too broad Changed to == pa.date32() — pins exact type, prevents silent regression to date64
test_datetime_tag_as_table_schema used pa.types.is_timestamp() — too broad Changed to == pa.timestamp("us", tz="UTC") — pins both precision and timezone per the converter contract
Typo "accomodate" in spec doc Fixed → "accommodate"
Typo "accomodate" in plan doc Fixed → "accommodate"

All 11 TestTagDatetimeValues tests continue to pass.

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.

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Comment thread test-objective/unit/test_semantic_types.py
@kurodo3

kurodo3 Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Review round (no code changes)

The single comment in this round was declined:

Comment Decision Reason
Remove unused timezone import ❌ Declined timezone is used in 5 places — every datetime-valued test constructs datetime(..., tzinfo=timezone.utc) (lines 196, 202, 210, 217, 239). Removing it would cause NameError in those tests.

No changes pushed.

@eywalker
eywalker merged commit 4fbb856 into main Jun 29, 2026
12 checks passed
@eywalker
eywalker deleted the eywalker/itl-45-add-support-for-datetime-in-tag-values branch June 29, 2026 03:39
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.

2 participants