Skip to content

test(page): pin nested-model serialization for to_dataframe / to_polars#130

Merged
TexasCoding merged 1 commit into
mainfrom
test/issue-101-dataframe-nested
May 17, 2026
Merged

test(page): pin nested-model serialization for to_dataframe / to_polars#130
TexasCoding merged 1 commit into
mainfrom
test/issue-101-dataframe-nested

Conversation

@TexasCoding

Copy link
Copy Markdown
Owner

Summary

Wave 2 #101: pins the behavior of Page.to_dataframe() / Page.to_polars() for nested-Pydantic-model fields, so a future pandas/polars upgrade can't silently change how nested structures land in cells.

6 new tests in `tests/test_page_dataframe.py` (12 total in this file now).

Observed behavior (now pinned)

pandas to_dataframe():

  • Nested Pydantic model → object column; each cell is the raw dict from model_dump(mode="python") (NOT expanded to inner.* columns, NOT stringified)
  • list[Decimal]object column holding Python list[Decimal]
  • Nested Decimal values stay Decimal (mode="python" contract held)

polars to_polars():

  • Nested model → Struct column (auto-inferred from dict shape); cell round-trips to dict with original types
  • list[Decimal]List(Decimal(precision=38, scale=N)) column
  • Top-level Decimal → Decimal dtype

No bug in Page. R-08 perf observation (intermediate dict allocations) noted in the issue body but out of scope for this PR.

Closes #101

Test plan

  • uv run pytest tests/test_page_dataframe.py -q — 12 passed (was 6)
  • Full unit suite: 1566 passed, 48 skipped
  • uv run ruff check . clean
  • uv run mypy kalshi/ clean

Adds 6 tests covering Page[T] DataFrame conversion when T has nested
Pydantic sub-models and list[Decimal] fields. Pins the current
model_dump(mode="python") contract so a future flip to mode="json", or
a pandas/polars upgrade that changes nested-cell inference, fails CI
instead of silently breaking downstream Decimal arithmetic.

Pinned behavior:
- pandas: nested model -> object column holding raw dict (Decimals
  preserved inside); list[Decimal] -> object column holding list of
  Decimal.
- polars: nested model -> Struct column; list[Decimal] -> List(Decimal)
  column.
- Top-level Decimal stays Decimal in both backends even when nested
  columns share the frame.

Closes #101

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented May 17, 2026

Copy link
Copy Markdown

Code Review — PR #130: Pin nested-model serialization for to_dataframe / to_polars

TL;DR: This is a clean, purposeful test-only PR. The tests are well-structured, comments explain the why correctly, and the PR scope is appropriately narrow. A few minor consistency gaps worth addressing before merge.


Overview

The PR adds 6 regression tests (12 total in the file) that pin the contract of Page.to_dataframe() / Page.to_polars() for nested Pydantic models, list[Decimal] fields, and Decimal preservation. No production code is changed. The stated goal — make a future mode="json" flip or pandas/polars upgrade fail loudly rather than silently — is sound and the test design achieves it.


What's Good

  • Comment quality is excellent. Inline comments explain why each assertion matters (mode="python" vs mode="json", what a Struct vs String column means). This is exactly the right use of comments per the project guidelines.
  • pytest.importorskip used consistently. Matches the pattern from the existing 6 tests above.
  • Scope is tight. The PR doesn't touch Page itself or other test files; every changed line traces directly to the issue.
  • PR description is clear — observed behavior, what's pinned, and what's explicitly out of scope (R-08 perf).

Issues & Suggestions

1. Missing df.shape assertion in all three TestToPolarsNested tests (minor inconsistency)

TestToDataframeNested asserts df.shape == (2, 4) in test_nested_model_lands_as_dict_object_column and df.columns. The polars equivalents check df.columns but skip the shape check. Makes the coverage asymmetric and could miss a silent row-count bug.

# add to each TestToPolarsNested test:
assert df.shape == (2, 4)
assert df.columns == ["ticker", "price", "inner", "prices"]

2. test_nested_model_lands_as_struct_column only verifies row 0 (minor gap)

first = df["inner"][0]
assert first == {"label": "x", "score": Decimal("0.10")}

Row 1 ({"label": "y", "score": Decimal("0.20")}) is never checked. Since the goal is to pin serialization shape, verifying both rows costs one line and removes the implicit assumption that polars handles the second row identically.

3. test_top_level_decimal_preserved_alongside_nested is redundant with TestToDataframe.test_happy_path (nit)

TestToDataframe.test_happy_path already pins top-level Decimal preservation. The new test adds value only insofar as it confirms the presence of nested columns in the same frame doesn't disturb top-level types. The test name should make that framing explicit, e.g., test_top_level_decimal_unaffected_by_nested_columns, so a future reader understands why this test exists alongside the existing happy-path coverage.

4. Repeated Page + to_dataframe() construction across all three pandas tests (style, not a bug)

Each TestToDataframeNested test independently calls Page(items=_sample_nested_items(), cursor=None) and then page.to_dataframe(). This is consistent with how the existing TestToDataframe class is written, so it's fine to leave as-is. Flagging only because a shared @pytest.fixture(scope="class") would be the natural evolution if more tests are added later.

5. _Inner / _NestedRow docstrings (nit)

class _Inner(BaseModel):
    """Nested sub-model — analog of OrderbookLevel inside Market."""

Single-line, explains the "why". This is within the project's one-short-line guideline. No change needed — just confirming it's fine.


Correctness Check

  • list[Decimal] in _NestedRow with from __future__ import annotations at the top of the file — works on Python 3.9+. ✓
  • pytest.importorskip returns the module, so pl = pytest.importorskip("polars") being unused in test_list_decimal_column_is_list_of_decimal_dtype and test_top_level_decimal_preserved_alongside_nested is fine (skip-on-missing is the intent). ✓
  • df["inner"][0] on a polars Struct column returns a dict — correct. ✓

Summary

The PR is ready to merge with one ask: add the missing df.shape + df.columns assertions to the three TestToPolarsNested tests (item 1 above) to bring parity with the pandas side. Items 2 and 3 are worth addressing if you want airtight coverage, but they're not blockers.

@TexasCoding TexasCoding merged commit e534eb4 into main May 17, 2026
4 checks passed
@TexasCoding TexasCoding deleted the test/issue-101-dataframe-nested branch May 17, 2026 16:07
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.

Test coverage: Page.to_dataframe / to_polars nested-model serialization unpinned

1 participant