fix(universal_converter): auto-register Pydantic/dataclass types in DictSource path (ITL-446)#213
Conversation
…stration (ITL-446)
…L-446) Removed unused pytest import and replaced meaningless 'pod is not None' assertion with assertion that verifies the pod was constructed with the expected output schema. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ython_dicts_to_arrow_table (ITL-446) Calls ensure_types_registered_for_schemas before converting Python schema to Arrow schema, enabling DictSource to automatically register Pydantic/dataclass column types without requiring manual pre-registration via a function pod. The method is idempotent and thread-safe, making it safe to call here. Also fixes a test bug where _Sensor was defined as a local class inside test_pod_registration_not_broken, causing get_type_hints to fail due to local scope resolution issues. Moved _Sensor to module-level definition like other test fixtures (_Point, _Measurement).
…ing and fix test comment (ITL-446) - Expanded python_dicts_to_arrow_table docstring to clarify auto-registration of Pydantic BaseModel and @DataClass types, including the idempotent nature of registration when arrow_schema is None - Updated test_dict_source_auto_registration.py comment block to reflect that classes are defined at module level for type hints resolution, with emphasis on idempotent registration behavior Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a DictSource failure mode where Pydantic BaseModel subclasses and @dataclass instances appearing as column values could raise an “Unsupported Python type” error because their types were not registered in the semantic type registry. The fix mirrors the existing function-pod eager-registration behavior by ensuring discovered schema types are registered during Python-dict → Arrow conversion.
Changes:
- Auto-register schema types in
UniversalTypeConverter.python_dicts_to_arrow_tablebefore deriving an Arrow schema from a Python schema. - Add a focused test suite covering Pydantic/dataclass “no prior registration”, idempotency, and “pod registration not broken”.
- Add design/spec + implementation plan documentation for the change.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/orcapod/semantic_types/universal_converter.py |
Registers schema types before Python-schema → Arrow-schema conversion in the arrow_schema is None path. |
tests/test_core/sources/test_dict_source_auto_registration.py |
Adds 4 tests validating DictSource auto-registration and compatibility with pod eager-registration. |
superpowers/specs/2026-07-07-dict-source-auto-registration-design.md |
Design note describing the rationale and intended behavior. |
superpowers/plans/2026-07-07-dict-source-auto-registration.md |
Step-by-step implementation plan and verification checklist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| arrow_schema: Optional Arrow schema. When provided the Python schema | ||
| is derived from it; no type registration is performed. |
There was a problem hiding this comment.
Good catch — the previous docstring conflated two distinct cases. Fixed in d4efa1c: the arrow_schema entry now explicitly documents both sub-cases:
python_schemaomitted → Python schema is derived fromarrow_schema; no type registration.python_schemasupplied → both schemas used as-is with a compatibility warning; no type registration in either case.
…ovided case (ITL-446)
Review round 1 — changes made1 comment addressed (commit d4efa1c):
|
| This uses the main conversion logic and caches results for performance. | ||
| When deriving the Arrow schema from a Python schema (i.e. when | ||
| ``arrow_schema`` is ``None``), Pydantic ``BaseModel`` subclasses and | ||
| ``@dataclass`` types found in the schema are automatically registered |
There was a problem hiding this comment.
This makes is sound specific to these but in reality this is for any factory-based logical type, and thus not fundamentally limited to Dataclass or Pydantic
There was a problem hiding this comment.
Good point — fixed in 3fd784b. Both the docstring intro and the inline code comment now refer to "any type that has a registered factory in the semantic-type system" rather than naming Pydantic/dataclass specifically. Pydantic and @dataclass are still mentioned in the inline comment as examples, but the framing makes clear they are not the only cases.
…s-on-dictsource-need-auto
…s, not just Pydantic/dataclass (ITL-446)
Review round 2 — changes made1 comment addressed (commit 3fd784b):
|
eywalker
deleted the
eywalker/itl-446-pydantic-dataclass-instances-on-dictsource-need-auto
branch
Summary
self.ensure_types_registered_for_schemas(python_schema)inUniversalTypeConverter.python_dicts_to_arrow_tableimmediately before converting the Python schema to Arrow. This auto-registers PydanticBaseModelsubclasses and@dataclasscolumn types encountered throughDictSourcewithout requiring prior function-pod registration.Test Plan
uv run pytest tests/test_core/sources/test_dict_source_auto_registration.py -v— all 4 new tests passuv run pytest tests/ -x -q— 4144 passed, 0 failures, no regressionsFixes ITL-446