fix: Do not convert nparray into list before wrapping into pandas.Series #23
Conversation
📝 WalkthroughWalkthroughThe pull request changes Sequence Diagram(s)Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
📦 Python package built successfully!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #23 +/- ##
==========================================
- Coverage 76.94% 75.39% -1.55%
==========================================
Files 99 99
Lines 5512 5625 +113
Branches 753 784 +31
==========================================
Hits 4241 4241
- Misses 1271 1384 +113 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
📒 Files selected for processing (1)
tests/unit/test_analyze_columns_pandas.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/unit/test_analyze_columns_pandas.py (1)
deepnote_toolkit/ocelots/pandas/analyze.py (1)
analyze_columns(102-200)
🪛 Ruff (0.14.3)
tests/unit/test_analyze_columns_pandas.py
599-599: Use a regular assert instead of unittest-style assertEqual
Replace assertEqual(...) with assert ...
(PT009)
600-600: Use a regular assert instead of unittest-style assertEqual
Replace assertEqual(...) with assert ...
(PT009)
601-601: Use a regular assert instead of unittest-style assertEqual
Replace assertEqual(...) with assert ...
(PT009)
602-602: Use a regular assert instead of unittest-style assertIsNotNone
Replace assertIsNotNone(...) with assert ...
(PT009)
603-603: Use a regular assert instead of unittest-style assertIsNotNone
Replace assertIsNotNone(...) with assert ...
(PT009)
604-604: Use a regular assert instead of unittest-style assertIsInstance
Replace assertIsInstance(...) with assert ...
(PT009)
605-605: Use a regular assert instead of unittest-style assertGreater
Replace assertGreater(...) with assert ...
(PT009)
607-607: Use a regular assert instead of unittest-style assertIn
Replace assertIn(...) with assert ...
(PT009)
608-608: Use a regular assert instead of unittest-style assertIn
Replace assertIn(...) with assert ...
(PT009)
627-627: Use a regular assert instead of unittest-style assertEqual
Replace assertEqual(...) with assert ...
(PT009)
628-628: Use a regular assert instead of unittest-style assertIsNotNone
Replace assertIsNotNone(...) with assert ...
(PT009)
629-629: Use a regular assert instead of unittest-style assertIsNotNone
Replace assertIsNotNone(...) with assert ...
(PT009)
632-632: Use a regular assert instead of unittest-style assertIn
Replace assertIn(...) with assert ...
(PT009)
637-637: Use a regular assert instead of unittest-style assertEqual
Replace assertEqual(...) with assert ...
(PT009)
653-653: Use a regular assert instead of unittest-style assertEqual
Replace assertEqual(...) with assert ...
(PT009)
654-654: Use a regular assert instead of unittest-style assertIsNotNone
Replace assertIsNotNone(...) with assert ...
(PT009)
655-655: Use a regular assert instead of unittest-style assertIsNotNone
Replace assertIsNotNone(...) with assert ...
(PT009)
656-656: Use a regular assert instead of unittest-style assertGreaterEqual
Replace assertGreaterEqual(...) with assert ...
(PT009)
657-657: Use a regular assert instead of unittest-style assertLessEqual
Replace assertLessEqual(...) with assert ...
(PT009)
660-660: Use a regular assert instead of unittest-style assertTrue
Replace assertTrue(...) with assert ...
(PT009)
⏰ 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: Build and push artifacts for Python 3.11
- GitHub Check: Build and push artifacts for Python 3.9
- GitHub Check: Build and push artifacts for Python 3.10
- GitHub Check: Build and push artifacts for Python 3.13
- GitHub Check: Build and push artifacts for Python 3.12
- GitHub Check: Test - Python 3.9
- GitHub Check: Test - Python 3.10
- GitHub Check: Test - Python 3.11
🔇 Additional comments (3)
tests/unit/test_analyze_columns_pandas.py (3)
5-5: LGTM!Import is necessary for the new Trino-specific tests.
610-637: LGTM!Test properly validates missing value handling with NamedRowTuple objects.
639-660: LGTM!Test correctly validates category aggregation with "others" bucket for many unique NamedRowTuple values.
|
🚀 Review App Deployment Started
|
|
You can test this with following Trino SQL query SELECT CAST(ROW(1, 'Alice') AS ROW(id INTEGER, name VARCHAR)) AS user
UNION ALL
SELECT CAST(ROW(2, 'Bob') AS ROW(id INTEGER, name VARCHAR)) AS userOr by constructing DF manually from trino.types import NamedRowTuple
import pandas as pd
import numpy as np
row1 = NamedRowTuple(values=[1, "Alice"], names=["id", "name"], types=["integer", "varchar"])
row2 = NamedRowTuple(values=[2, "Bob"], names=["id", "name"], types=["integer", "varchar"])
np_array = np.empty(2, dtype=object)
np_array[0] = row1
np_array[1] = row2
df = pd.DataFrame({"col1": np_array})
df |
m1so
deleted the
oleh/blu-5137-column-analysis-fails-on-trinotypesnamedrowtuple
branch
Trying to construct pandas Series from a list of
trino.types.NamedRowTuplefails, but works perfectly fine if we pass numpy array directly.It's also caused by incorrect implementation of
__getattr__leading NumPy/Pandas to believe it supports array struct protocol:__array_struct__,__getattr__raisesAttributeErrorandhasattr()returnsFalse__array_struct__,__getattr__returnsNone(doesn't raiseAttributeError) andhasattr()returnsTrueTraceback
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests