Add native TimeOnly to time conversion#802
Add native TimeOnly to time conversion#802buvinghausen wants to merge 1 commit intotonybaloney:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds first-class TimeOnly ↔ Python datetime.time interoperability across the source generator (parsing/type reflection/signature generation) and runtime (PyObject conversions/importers), plus related unit/snapshot coverage and new Python integration module.
Changes:
- Introduces a
TimeTypein the type system and parses"time"/"datetime.time"annotations to it. - Maps
TimeTypeto C#TimeOnlyin reflection/signature generation and adds result conversion support (includingOptional[time]→TimeOnly?). - Adds runtime conversions/importer +
PyObject.From(TimeOnly)overloads and updates PublicAPI declarations; adds Python-side functions and generator snapshot updates.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Integration.Tests/python/test_time.py | Adds Python functions for datetime.time roundtrip/creation. |
| src/CSnakes.Tests/TypeReflectionTests.cs | Verifies predefined type mapping for time/datetime.time → TimeOnly. |
| src/CSnakes.Tests/PythonTypeSpecTests.cs | Asserts PythonTypeSpec.Time singleton and TimeType behavior. |
| src/CSnakes.Tests/PythonTypeDefinitionParserTests.cs | Adds parser tests for time and datetime.time. |
| src/CSnakes.Tests/PythonStaticGeneratorTests/FormatClassFromMethods.test_time.approved.txt | Updates approved snapshot showing generated TimeOnly signatures/importers. |
| src/CSnakes.Tests/GeneratedSignatureTests.cs | Adds signature generation test for time/datetime.time usage. |
| src/CSnakes.SourceGeneration/ResultConversionCodeGenerator.cs | Adds scalar/optional result conversion support for TimeType. |
| src/CSnakes.SourceGeneration/Reflection/TypeReflection.cs | Maps TimeType to TimeOnly for predefined type reflection. |
| src/CSnakes.SourceGeneration/Parser/Types/PythonTypeSpec.cs | Adds TimeType + PythonTypeSpec.Time. |
| src/CSnakes.SourceGeneration/Parser/PythonParser.TypeDef.cs | Parses "time" / "datetime.time" into TimeType. |
| src/CSnakes.Runtime/Python/PyObjectImporters.cs | Adds PyObjectImporters.Time : IPyObjectImporter<TimeOnly>. |
| src/CSnakes.Runtime/Python/PyObject.cs | Adds PyObject.From(TimeOnly) / From(TimeOnly?) and As(Type) support for TimeOnly. |
| src/CSnakes.Runtime/PyObjectTypeConverter.Time.cs | Implements TimeOnly ↔ datetime.time conversion via CPython API. |
| src/CSnakes.Runtime/PublicAPI/net8.0/PublicAPI.Unshipped.txt | Declares new public API surface for importer and From(TimeOnly) overloads. |
| src/CSnakes.Runtime/PublicAPI/net9.0/PublicAPI.Unshipped.txt | Declares new public API surface for importer and From(TimeOnly) overloads. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| internal static TimeOnly ConvertToTimeOnly(PyObject pyObject) | ||
| { | ||
| using PyObject hourObj = pyObject.GetAttr("hour"); | ||
| using PyObject minuteObj = pyObject.GetAttr("minute"); | ||
| using PyObject secondObj = pyObject.GetAttr("second"); | ||
| using PyObject microsecondObj = pyObject.GetAttr("microsecond"); | ||
|
|
||
| long us = CPythonAPI.PyLong_AsLongLong(microsecondObj); | ||
|
|
||
| return new TimeOnly( | ||
| (int)CPythonAPI.PyLong_AsLongLong(hourObj), | ||
| (int)CPythonAPI.PyLong_AsLongLong(minuteObj), | ||
| (int)CPythonAPI.PyLong_AsLongLong(secondObj), | ||
| millisecond: (int)(us / 1000), | ||
| microsecond: (int)(us % 1000)); |
There was a problem hiding this comment.
ConvertToTimeOnly reconstructs a TimeOnly from hour/minute/second/microsecond but ignores datetime.time's tzinfo (and fold) fields. This can silently drop timezone/ambiguity information when an aware datetime.time is passed in. Consider explicitly checking tzinfo is None (and optionally fold == 0) and throwing a clear exception if unsupported values are present, so callers don’t get lossy conversions without noticing.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| internal static PyObject ConvertFromTimeOnly(TimeOnly time) | ||
| { | ||
| using PyObject datetimeModule = CPythonAPI.Import("datetime"); | ||
| using PyObject timeClass = datetimeModule.GetAttr("time"); | ||
| using PyObject hourArg = PyObject.Create(CPythonAPI.PyLong_FromLongLong(time.Hour)); | ||
| using PyObject minuteArg = PyObject.Create(CPythonAPI.PyLong_FromLongLong(time.Minute)); | ||
| using PyObject secondArg = PyObject.Create(CPythonAPI.PyLong_FromLongLong(time.Second)); | ||
| // Python microsecond is 0–999999; C# splits ms and us | ||
| using PyObject microsecondArg = PyObject.Create( | ||
| CPythonAPI.PyLong_FromLongLong(time.Millisecond * 1000L + time.Microsecond)); | ||
| return timeClass.Call(hourArg, minuteArg, secondArg, microsecondArg); | ||
| } | ||
|
|
||
| internal static TimeOnly ConvertToTimeOnly(PyObject pyObject) | ||
| { | ||
| using PyObject hourObj = pyObject.GetAttr("hour"); | ||
| using PyObject minuteObj = pyObject.GetAttr("minute"); | ||
| using PyObject secondObj = pyObject.GetAttr("second"); | ||
| using PyObject microsecondObj = pyObject.GetAttr("microsecond"); | ||
|
|
||
| long us = CPythonAPI.PyLong_AsLongLong(microsecondObj); | ||
|
|
||
| return new TimeOnly( | ||
| (int)CPythonAPI.PyLong_AsLongLong(hourObj), | ||
| (int)CPythonAPI.PyLong_AsLongLong(minuteObj), | ||
| (int)CPythonAPI.PyLong_AsLongLong(secondObj), | ||
| millisecond: (int)(us / 1000), | ||
| microsecond: (int)(us % 1000)); | ||
| } |
There was a problem hiding this comment.
The new TimeOnly ↔ datetime.time runtime conversion path (via ConvertFromTimeOnly / ConvertToTimeOnly and PyObject.From(TimeOnly)) doesn’t appear to be exercised by any C# integration test (there are no Env.TestTime() usages in src/Integration.Tests). Consider adding an Integration.Tests/TimeTests.cs (similar to BasicTests/BufferTests) that calls into test_time.py and asserts round-trip values, including microsecond boundaries (e.g., 0, 999999) and that sub-microsecond ticks are dropped as documented.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Add
TimeOnly↔datetime.timeinteropSummary
Adds first-class support for Python's
datetime.timetype, mapping it to C#'sTimeOnly. Follows the same pattern established byDateOnly↔datetime.date.Conversion nuances
TimeOnlystores sub-second precision as separateMillisecond(0–999) andMicrosecond(0–999) fields, plus aTicksremainder that has no Python equivalent.datetime.timestores a singlemicrosecondfield in the range 0–999999.us_total = ms * 1000 + uson the way to Python, andms = us_total / 1000,us = us_total % 1000on the way back. Sub-microsecond ticks are silently dropped, which is acceptable since Python has no way to represent them.Changes
CSnakes.SourceGenerationParser/Types/PythonTypeSpec.cs— AddedTimeTypesingleton record andPythonTypeSpec.Timestatic property.Parser/PythonParser.TypeDef.cs— AddedTimesub-parser and mapped"time"/"datetime.time"tokens in the type definition switch.Reflection/TypeReflection.cs— Added(TimeType, _, _) => [ParseTypeName("TimeOnly")]toAsPredefinedType.ResultConversionCodeGenerator.cs— Added staticTimescalar generator field, aTimeTypecase inCreate(), and addedTimeTypeto the value-type optional pattern soOptional[datetime.time]maps toTimeOnly?viaOptionalValue.CSnakes.RuntimePyObjectTypeConverter.Time.cs(new) —ConvertFromTimeOnlyconstructs adatetime.timevia the CPython API, combining C#'sMillisecondandMicrosecondinto Python's singlemicrosecondargument.ConvertToTimeOnlyreads the four attributes back and reconstructs aTimeOnly.Python/PyObjectImporters.cs— AddedTime : IPyObjectImporter<TimeOnly>sealed class delegating toConvertToTimeOnly.Python/PyObject.cs— AddedFrom(TimeOnly)/From(TimeOnly?)overloads and aTimeOnlybranch in theAs(Type)switch.PublicAPI/net8.0/PublicAPI.Unshipped.txtandPublicAPI/net9.0/PublicAPI.Unshipped.txt— DeclaredPyObjectImporters.Timeand bothFrom(TimeOnly)overloads.Tests
PythonTypeDefinitionParserTests.cs— AddedTimeTesttheory for"time"/"datetime.time"→TimeType.TypeReflectionTests.cs— Added("time", "TimeOnly")and("datetime.time", "TimeOnly")toAsPredefinedTypeData.PythonTypeSpecTests.cs— AddedTimeTypeto the singletons assertion and aTimeTypeTestsinner class.GeneratedSignatureTests.cs— Addedtime/datetime.timeround-trip signature test cases.Integration.Tests/python/test_time.py(new) —test_time_roundtripandtest_create_timePython functions.PythonStaticGeneratorTests/FormatClassFromMethods.test_time.approved.txt(new) — Approval snapshot for the generated C# wrapper.