feat: extend set[T] round-trip to support primitive element types (ITL-611) - #258
Conversation
…element round-trip
…t type support
- Add _NATIVE_ELEMENT_TYPES dict (int, str, float, bool, bytes, datetime, date)
- Add _get_native_element_arrow_type() helper
- Unify __init__ via duck-typing: extension mode when element has
get_arrow_extension_type(), native mode for plain Python types
- Native metadata format: {category, element_kind: "native", element_python_type}
- reconstruct_from_arrow: add native branch before extension-mode logic
- create_for_python_type: handle set[T] for native T
…s (ITL-611) Add _make_or_get_native_list_logical_type helper; update both set[T] branches in _register_python_class_impl and _convert_python_to_arrow to wrap native element types (int, str, float, bool, bytes, datetime, date) in ListLogicalType. list[T] for primitive T remains plain large_list (unchanged).
…TL-611 Covers set[int|str|float|bool|bytes|datetime] through Parquet and Delta backends, schema reconstruction, list[int] regression guard, explicit ListLogicalType(int, is_set=False) construction, and fresh-converter read-back without prior registration.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Implements ITL-611 by extending ListLogicalType and the universal type converter so set[T] with primitive/native T (e.g., set[int]) round-trips through Arrow/Parquet/Delta while preserving set semantics via an Arrow extension type.
Changes:
- Added a native-element mode to
ListLogicalType(metadata discriminator + native type map) and updated factory reconstruction to support it. - Updated
UniversalTypeConverterset[T]handling to wrap primitiveTin a nativeListLogicalTypeextension type. - Added unit + round-trip tests covering native-mode behavior and set round-trips for several primitive element types.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/orcapod/logical_types/list_logical_type_factory.py |
Adds native element support, metadata format, and reconstruct logic for set[T]/list[T]. |
src/orcapod/semantic_types/universal_converter.py |
Wraps primitive set[T] as native ListLogicalType to preserve set semantics. |
tests/test_logical_types/test_list_logical_type.py |
Adds native-mode constructor + factory reconstruction unit tests. |
tests/test_logical_types/test_roundtrips.py |
Adds converter write-path checks and storage round-trip tests for native set[T]. |
superpowers/specs/2026-08-24-itl-611-set-any-element-type-design.md |
Design spec capturing the native-mode approach and metadata format. |
superpowers/plans/2026-08-24-itl-611-set-any-element-type.md |
Implementation plan detailing steps, code changes, and test coverage. |
Suppressed comments (1)
src/orcapod/logical_types/list_logical_type_factory.py:386
- This error message is now misleading: native
set[T]is supported for specific primitive element types, but the message still implies primitive element types are unsupported in all cases. Clarify the supported cases (and list which native element types are accepted) so failures are actionable.
f"ListLogicalTypeFactory.create_for_python_type: element type "
f"{element_annotation!r} has no registered LogicalType. "
f"Only list[T]/set[T] where T maps to a LogicalType are supported; "
f"use plain list[{element_annotation}] for primitive element types."
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if element_kind == "native": | ||
| # Native mode: element is a plain Python type from _NATIVE_ELEMENT_TYPES. | ||
| element_python_type_name = metadata.get("element_python_type") | ||
| if not element_python_type_name: | ||
| raise ValueError( | ||
| f"ListLogicalTypeFactory.reconstruct_from_arrow: missing " | ||
| f"'element_python_type' in native-mode metadata for " | ||
| f"{arrow_extension_name!r}. metadata={metadata!r}." | ||
| ) | ||
| element_python_type = _NATIVE_ELEMENT_TYPES.get(element_python_type_name) | ||
| if element_python_type is None: | ||
| raise ValueError( | ||
| f"ListLogicalTypeFactory.reconstruct_from_arrow: unknown native element " | ||
| f"type {element_python_type_name!r} for {arrow_extension_name!r}. " | ||
| f"Supported: {list(_NATIVE_ELEMENT_TYPES.keys())!r}." | ||
| ) | ||
| logger.debug( | ||
| "ListLogicalTypeFactory: reconstructed %r from Arrow as native mode (is_set=%s)", | ||
| arrow_extension_name, | ||
| is_set, | ||
| ) | ||
| return ListLogicalType(element_python_type, is_set=is_set) |
There was a problem hiding this comment.
Fixed. In reconstruct_from_arrow, after resolving element_python_type from metadata, we now call _get_native_element_arrow_type(element_python_type) to derive the expected Arrow value type, then compare it against storage_type.value_type using _canonical_storage (from the registry module) to handle Delta Lake's string/large_string normalization. A mismatch raises ValueError with a detailed message identifying the extension name, the claimed element type, the expected value type, and the actual value type.
A corresponding test (test_list_logical_type_factory_reconstruct_native_storage_type_mismatch_raises) was added in tests/test_logical_types/test_list_logical_type.py — it builds a large_list(large_string) storage type while metadata claims element_python_type='int' and asserts a ValueError with message matching "storage type mismatch".
| def test_set_of_datetime_round_trip(storage_backend: _StorageBackend, tmp_path: Path) -> None: | ||
| """set[datetime] values round-trip as sets of timezone-aware datetimes.""" | ||
| from datetime import datetime, timezone | ||
| dt1 = datetime(2024, 1, 1, 0, 0, 0, tzinfo=timezone.utc) | ||
| dt2 = datetime(2024, 6, 15, 12, 30, 0, tzinfo=timezone.utc) | ||
| data = {dt1, dt2} | ||
| result, read_converter = _write_and_read( | ||
| {"timestamps": set[datetime]}, | ||
| [{"timestamps": data}], | ||
| storage_backend, | ||
| tmp_path, | ||
| ) | ||
| field = result.schema.field("timestamps") | ||
| assert hasattr(field.type, "extension_name") | ||
| assert field.type.extension_name == "set[datetime]" | ||
| rows = read_converter.arrow_table_to_python_dicts(result) | ||
| assert isinstance(rows[0]["timestamps"], set) | ||
| assert rows[0]["timestamps"] == data | ||
|
|
||
|
|
There was a problem hiding this comment.
Fixed. Added test_set_of_date_round_trip to tests/test_logical_types/test_roundtrips.py parametrized over both Parquet and Delta backends. The test writes a row with a set[date] column containing three date values, reads it back, and asserts that (1) the Arrow field carries the "set[date]" extension name and (2) the round-tripped value equals the original Python set of date objects.
…n; fix error message; add set[date] test - In reconstruct_from_arrow, validate that the Arrow storage value type matches what the metadata claims (e.g. metadata says int but field has large_string raises ValueError). Uses _canonical_storage to handle Delta Lake's string/large_string normalization. - Fix misleading error in create_for_python_type that said set[T] with native primitive T was unsupported; message now lists the supported native types explicitly. - Add test_list_logical_type_factory_reconstruct_native_storage_type_mismatch_raises to test_list_logical_type.py covering the new validation. - Add test_set_of_date_round_trip to test_roundtrips.py (Parquet + Delta) ensuring date, a non-datetime native type, round-trips correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Response to Copilot reviewAddressed all three review items in commit 46dbaa1. Summary of changes: 1. Storage-type validation in
|
…ork-for-any-element-type-not-just
Summary
ListLogicalTypewith a native element mode: the constructor now accepts a plain Python type (e.g.int) alongsideLogicalTypeProtocol, using duck-typing dispatch to distinguish the two modes._NATIVE_ELEMENT_TYPESdict (7 types:int,str,float,bool,bytes,datetime,date) and_get_native_element_arrow_type()helper.{"category": "set", "element_kind": "native", "element_python_type": "int"}— backward-compatible with existing extension-mode metadata.ListLogicalTypeFactory.reconstruct_from_arrowto handle native mode on the read path._make_or_get_native_list_logical_typehelper toUniversalTypeConverter; updatesset[T]branches to wrap primitiveTin a nativeListLogicalType.list[T]for primitiveTis unchanged — still produces plainpa.large_list(T).Closes ITL-611
Test plan
ListLogicalTypeconstructor (all 7 types, metadata format, storage conversions)reconstruct_from_arrownative branch (success + error cases)set[int]/set[str]produce extension type,list[int]unchanged regression)set[int]→ Arrow schema →set[int](notlist[int]orset[Any])set[int|str|float|bool|bytes|datetime]🤖 Generated with Claude Code