Skip to content

Conversation

@m1so
Copy link
Member

@m1so m1so commented Nov 7, 2025

Summary by CodeRabbit

  • Tests
    • Strengthened test infrastructure with comprehensive integration tests for Trino STRUCT/ROW and ARRAY types, validating serialization, field access patterns, and data analysis capabilities
    • Enhanced DataFrame rendering test suite with extensive coverage for structured data types including nested objects, JSON conversion, and statistical analysis, ensuring data integrity and proper visualization

@linear
Copy link

linear bot commented Nov 7, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

📝 Walkthrough

Walkthrough

Two test suites are added to strengthen type-handling validation. Integration tests in test_trino.py verify correct behavior when Trino STRUCT/ROW and ARRAY types are executed and converted—checking column presence, field access patterns (index and attribute), and JSON serialization. Unit tests in test_dataframe_rendering.py comprehensively validate DataFrame rendering of structured data across multiple scenarios: dicts, lists, tuples, NamedRowTuple instances, nested structures, and mixed types. Both suites confirm that analyze_columns() and to_records() handle complex types without crashes and produce JSON-serializable output.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR title 'chore: Add regression tests for DataFrame rendering and analysis' accurately describes the main changes—adding integration and unit tests for structured types.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Nov 7, 2025

📦 Python package built successfully!

  • Version: 1.1.2.dev5+4209060
  • Wheel: deepnote_toolkit-1.1.2.dev5+4209060-py3-none-any.whl
  • Install:
    pip install "deepnote-toolkit @ https://deepnote-staging-runtime-artifactory.s3.amazonaws.com/deepnote-toolkit-packages/1.1.2.dev5%2B4209060/deepnote_toolkit-1.1.2.dev5%2B4209060-py3-none-any.whl"

@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.39%. Comparing base (663dce1) to head (4b40a57).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #25   +/-   ##
=======================================
  Coverage   75.39%   75.39%           
=======================================
  Files          99       99           
  Lines        5625     5625           
  Branches      784      784           
=======================================
  Hits         4241     4241           
  Misses       1384     1384           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 7, 2025
@deepnote-bot
Copy link

deepnote-bot commented Nov 7, 2025

🚀 Review App Deployment Started

📝 Description 🌐 Link / Info
🌍 Review application ra-25
🔑 Sign-in URL Click to sign-in
📊 Application logs View logs
🔄 Actions Click to redeploy
🚀 ArgoCD deployment View deployment
Last deployed 2025-11-07 15:13:15 (UTC)
📜 Deployed commit f7f7cb644b2cdd2dab6442e529e6cd654f5fe25e
🛠️ Toolkit version 4209060

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d011e7b and 6f4b085.

📒 Files selected for processing (1)
  • tests/unit/test_ocelots_pandas_utils.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unit/test_ocelots_pandas_utils.py (1)
deepnote_toolkit/ocelots/pandas/utils.py (1)
  • safe_convert_to_string (10-35)
🪛 Ruff (0.14.3)
tests/unit/test_ocelots_pandas_utils.py

49-49: Boolean positional value in function call

(FBT003)


57-57: Missing return type annotation for special method __str__

Add return type annotation: str

(ANN204)


58-58: Avoid specifying long messages outside the exception class

(TRY003)


60-60: Missing return type annotation for special method __repr__

Add return type annotation: str

(ANN204)


61-61: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Test - Python 3.10
  • GitHub Check: Test - Python 3.11
  • GitHub Check: Test - Python 3.9
  • GitHub Check: Build and push artifacts for Python 3.9
  • GitHub Check: Build and push artifacts for Python 3.11
  • GitHub Check: Build and push artifacts for Python 3.13
  • GitHub Check: Build and push artifacts for Python 3.12
  • GitHub Check: Build and push artifacts for Python 3.10
🔇 Additional comments (10)
tests/unit/test_ocelots_pandas_utils.py (10)

1-6: LGTM - Clean imports.

Necessary imports for testing JSON conversion and Trino types.


8-14: LGTM - Solid dict test.

Validates JSON output and round-trip parsing.


17-23: LGTM - Correct tuple→JSON→list handling.

Properly expects list after JSON round-trip (JSON lacks tuple type).


26-32: LGTM - Basic list coverage.


35-40: LGTM - Nested structure coverage.

Critical for complex Trino STRUCT/ARRAY types.


43-51: LGTM - Primitive type coverage.

Static analysis warning on line 49 is a false positive for test code.


54-64: LGTM - Error handling validated.

Static analysis warnings are false positives for test helper class.


68-84: LGTM - NamedRowTuple coverage.

Field access checks (lines 83-84) provide extra validation of Trino type behavior.


87-100: LGTM - Quote validation is excellent.

Lines 99-100 explicitly verify double-quote JSON vs single-quote str() output.


103-113: LGTM - Nested array coverage.

@m1so m1so changed the title fix(trino): convert STRUCT/ROW/ARRAY types to valid JSON strings chore: Add regression tests for DataFrame rendering and analysis Nov 7, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6f4b085 and 4b40a57.

📒 Files selected for processing (2)
  • tests/integration/test_trino.py (1 hunks)
  • tests/unit/test_dataframe_rendering.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unit/test_dataframe_rendering.py (1)
deepnote_toolkit/ocelots/pyspark/implementation.py (1)
  • to_records (223-282)
tests/integration/test_trino.py (2)
deepnote_toolkit/sql/sql_execution.py (1)
  • execute_sql (197-233)
deepnote_toolkit/ocelots/pyspark/implementation.py (2)
  • columns (30-37)
  • to_records (223-282)
🪛 Ruff (0.14.3)
tests/unit/test_dataframe_rendering.py

20-20: Missing return type annotation for private function _test_rendering_flow

Add return type annotation: None

(ANN202)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Test - Python 3.9
  • GitHub Check: Test - Python 3.11
  • GitHub Check: Test - Python 3.10
  • GitHub Check: Build and push artifacts for Python 3.9
  • GitHub Check: Build and push artifacts for Python 3.13
  • GitHub Check: Build and push artifacts for Python 3.11
  • GitHub Check: Build and push artifacts for Python 3.10
  • GitHub Check: Build and push artifacts for Python 3.12

@m1so m1so marked this pull request as ready for review November 7, 2025 15:21
@m1so m1so requested a review from a team as a code owner November 7, 2025 15:21
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.

3 participants