Skip to content

test(ITL-10): add dataclass → DataFrame conversion test coverage#216

Merged
eywalker merged 8 commits into
mainfrom
eywalker/itl-10-bug-dataclass-encoded-column-fails-to-convert-to-dataframe
Jul 8, 2026
Merged

test(ITL-10): add dataclass → DataFrame conversion test coverage#216
eywalker merged 8 commits into
mainfrom
eywalker/itl-10-bug-dataclass-encoded-column-fails-to-convert-to-dataframe

Conversation

@kurodo3

@kurodo3 kurodo3 Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds 6 tests (2 new files) confirming that dataclass-output FunctionNodes produce correct Polars DataFrames
  • Adds converter-level unit tests confirming plain Arrow structs round-trip safely through the converter as dynamic TypedDicts
  • No production code changes — the core bug (FN1) was already fixed; this PR closes ITL-10 by adding the required test coverage

Staleness finding

The exact reproducer from ITL-10 already runs without error. as_table() now derives its schema from output_schema() directly (FN1 fix), so the original ValueError: Unsupported Python type: typing.Any crash no longer occurs. arrow_schema_to_python_schema correctly returns the original dataclass type (not Any) when the extension type wrapper is present. When a plain Arrow struct (no extension metadata) is passed to arrow_schema_to_python_schema, the converter creates a dynamic TypedDict that round-trips back to the original struct type.

Files added

  • tests/test_core/nodes/test_function_node_dataclass.py — 3 end-to-end pipeline tests
  • tests/test_extension_types/test_universal_converter_struct.py — 3 converter-level unit tests

Fixes ITL-10

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@eywalker eywalker requested a review from Copilot July 8, 2026 03:52

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 adds regression test coverage for ITL-10, ensuring dataclass-returning FunctionNodes produce stable, correctly-typed Polars DataFrames via .as_df(), and that plain Arrow struct schemas are inferred as dynamic TypedDicts that round-trip back to the identical Arrow struct type through the universal converter.

Changes:

  • Added end-to-end pipeline tests validating dataclass output → as_df() behavior for populated and unrun nodes (including schema stability).
  • Added converter-level unit tests verifying plain Arrow struct → dynamic TypedDict inference, round-trip type mapping, and write-back table construction.
  • Added internal design/spec and implementation plan documentation for the ITL-10 test work.

Reviewed changes

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

File Description
tests/test_extension_types/test_universal_converter_struct.py New unit tests for struct → dynamic TypedDict inference + round-trip/write-back behavior.
tests/test_core/nodes/test_function_node_dataclass.py New end-to-end tests for dataclass-returning FunctionNode .as_df() output and schema stability.
superpowers/specs/2026-07-08-itl-10-dataclass-dataframe-conversion-tests-design.md New design/spec documenting ITL-10 staleness assessment and success criteria for tests.
docs/metamorphic/plans/2026-07-08-itl-10-dataclass-dataframe-tests.md New implementation plan documenting the steps to add/verify the new tests.

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

Comment on lines +32 to +35
@pytest.fixture
def converter():
"""Fresh UniversalTypeConverter from the default DataContext."""
return get_default_context().type_converter

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.

Fixed. The docstring now accurately states that get_default_context() returns a cached singleton and that the converter may be shared across tests. Added a note that the tests are written to be order-independent under that constraint.

@kurodo3

kurodo3 Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Review response (round 1)

One comment addressed:

  • Fixture docstring (): Removed the misleading word "Fresh". The docstring now accurately states that get_default_context() returns a cached singleton and that the converter instance may be shared across tests. Added a note that the tests are written to be order-independent under that constraint. (commit f58e35d)

@eywalker eywalker merged commit 96e3e96 into main Jul 8, 2026
11 checks passed
@eywalker eywalker deleted the eywalker/itl-10-bug-dataclass-encoded-column-fails-to-convert-to-dataframe branch July 8, 2026 05:51
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