Skip to content

Commit 4ecdbd1

Browse files
committed
Numpy test and null handling fixes
1 parent c9603e0 commit 4ecdbd1

13 files changed

Lines changed: 215 additions & 130 deletions

e2e_playwright/e2e_utils.py

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
INTERACTIONS_SCRIPT = Path(__file__).parent / "pivot_table_interactions_app.py"
4545
DATA_SCRIPT = Path(__file__).parent / "pivot_table_data_app.py"
4646
GOLDEN_SCRIPT = Path(__file__).parent / "pivot_table_golden_app.py"
47-
NUMPY_INPUT_SCRIPT = Path(__file__).parent / "pivot_table_numpy_input_app.py"
4847

4948
PIVOT_KEYS = [
5049
"test_pivot",
@@ -82,6 +81,8 @@
8281
"test_pivot_subtotals",
8382
"test_pivot_cond_fmt",
8483
"test_pivot_scalar_roundtrip",
84+
"test_pivot_numpy_array",
85+
"test_pivot_numpy_list",
8586
]
8687

8788
# Must match the exact order of st_pivot_table(key=...) calls in
@@ -144,11 +145,6 @@
144145
"golden_export",
145146
]
146147

147-
NUMPY_INPUT_PIVOT_KEYS = [
148-
"numpy_array_pivot",
149-
"numpy_str_list_pivot",
150-
]
151-
152148
APP_CONFIGS = {
153149
"default": {"script": SCRIPT, "pivot_keys": PIVOT_KEYS},
154150
"pivot_table_test.py": {"script": TOOLBAR_SCRIPT, "pivot_keys": TOOLBAR_PIVOT_KEYS},
@@ -161,10 +157,6 @@
161157
"script": GOLDEN_SCRIPT,
162158
"pivot_keys": GOLDEN_PIVOT_KEYS,
163159
},
164-
"pivot_table_numpy_input_test.py": {
165-
"script": NUMPY_INPUT_SCRIPT,
166-
"pivot_keys": NUMPY_INPUT_PIVOT_KEYS,
167-
},
168160
}
169161

170162

e2e_playwright/pivot_table_numpy_input_app.py

Lines changed: 0 additions & 62 deletions
This file was deleted.

e2e_playwright/pivot_table_numpy_input_test.py

Lines changed: 0 additions & 52 deletions
This file was deleted.

e2e_playwright/pivot_table_test.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,39 @@ def test_scalar_aggregation_roundtrip_persists_and_python_override_wins(
350350
)
351351

352352

353+
def test_numpy_backed_configs_render_without_frontend_validation_errors(
354+
page_at_app: Page,
355+
):
356+
"""NumPy-backed config inputs render like normal string-list configs."""
357+
page = page_at_app
358+
359+
raw_array = get_pivot(page, "test_pivot_numpy_array")
360+
expect(raw_array.get_by_test_id("pivot-table")).to_be_visible(timeout=15000)
361+
expect(raw_array.get_by_test_id("pivot-data-cell").first).to_be_visible(
362+
timeout=5000
363+
)
364+
expect(raw_array.get_by_test_id("toolbar-rows-chips")).to_contain_text("Region")
365+
expect(raw_array.get_by_test_id("toolbar-columns-chips")).to_contain_text("Year")
366+
expect(raw_array.get_by_test_id("toolbar-values-chips")).to_contain_text("Revenue")
367+
368+
numpy_str_list = get_pivot(page, "test_pivot_numpy_list")
369+
expect(numpy_str_list.get_by_test_id("pivot-table")).to_be_visible(timeout=15000)
370+
expect(numpy_str_list.get_by_test_id("pivot-data-cell").first).to_be_visible(
371+
timeout=5000
372+
)
373+
expect(numpy_str_list.get_by_test_id("toolbar-rows-chips")).to_contain_text(
374+
"Region"
375+
)
376+
expect(numpy_str_list.get_by_test_id("toolbar-columns-chips")).to_contain_text(
377+
"Year"
378+
)
379+
expect(numpy_str_list.get_by_test_id("toolbar-values-chips")).to_contain_text(
380+
"Revenue"
381+
)
382+
383+
expect(page.locator("text=must contain only strings")).to_have_count(0)
384+
385+
353386
def test_toolbar_swap_rows_columns(page_at_app: Page):
354387
"""Swapping rows and columns transposes the pivot layout."""
355388
page = page_at_app

e2e_playwright/pivot_table_toolbar_app.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717

1818
from __future__ import annotations
1919

20+
from typing import Any, cast
21+
22+
import numpy as np
2023
import streamlit as st
2124

2225
from streamlit_pivot import st_pivot_table
@@ -124,6 +127,34 @@ def render_app(data):
124127
on_config_change=noop,
125128
)
126129

130+
raw_rows = cast(Any, np.array(["Region"]))
131+
raw_columns = cast(Any, np.array(["Year"]))
132+
raw_values = cast(Any, np.array(["Revenue"]))
133+
134+
st.subheader("NumPy ndarray Pivot")
135+
st_pivot_table(
136+
df,
137+
key="test_pivot_numpy_array",
138+
rows=raw_rows,
139+
columns=raw_columns,
140+
values=raw_values,
141+
aggregation="sum",
142+
interactive=True,
143+
on_config_change=noop,
144+
)
145+
146+
st.subheader("NumPy list Pivot")
147+
st_pivot_table(
148+
df,
149+
key="test_pivot_numpy_list",
150+
rows=list(np.array(["Region"])),
151+
columns=list(np.array(["Year"])),
152+
values=list(np.array(["Revenue"])),
153+
aggregation="sum",
154+
interactive=True,
155+
on_config_change=noop,
156+
)
157+
127158

128159
def main():
129160
init_page()

streamlit_pivot/__init__.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,11 +375,14 @@ def _prepare_threshold_hybrid_frame(
375375
named: dict[str, pd.NamedAgg] = {}
376376
avg_fields: list[str] = []
377377
numeric_coerce_fields: list[str] = []
378+
count_like_fields: list[str] = []
378379

379380
for vf in value_fields:
380381
agg = aggregation.get(vf, "sum")
381382
if agg in _NUMERIC_COERCE_AGGS:
382383
numeric_coerce_fields.append(vf)
384+
if agg in ("count", "count_distinct"):
385+
count_like_fields.append(vf)
383386
if agg == "avg":
384387
avg_fields.append(vf)
385388
named[f"{vf}__sum"] = pd.NamedAgg(column=vf, aggfunc="sum")
@@ -412,6 +415,8 @@ def _prepare_threshold_hybrid_frame(
412415
ser = filtered_df[vf]
413416
if agg in _NUMERIC_COERCE_AGGS:
414417
ser = _coerce_measure_series(ser, vf, null_handling)
418+
elif agg in ("count", "count_distinct"):
419+
ser = _resolve_count_series(ser, vf, null_handling)
415420
if agg == "avg":
416421
cnt = int(ser.count())
417422
row[vf] = float(ser.sum() / cnt) if cnt else float("nan")
@@ -452,6 +457,8 @@ def _prepare_threshold_hybrid_frame(
452457
working[dim] = _resolve_dim_value_series(working[dim], col_type, mode, grain)
453458
for vf in numeric_coerce_fields:
454459
working[vf] = _coerce_measure_series(working[vf], vf, null_handling)
460+
for vf in count_like_fields:
461+
working[vf] = _resolve_count_series(working[vf], vf, null_handling)
455462

456463
out = (
457464
working.groupby(group_fields, dropna=False, observed=True, sort=False)
@@ -663,6 +670,13 @@ def _coerce_measure_series(series: Any, field: str, null_handling: Any) -> Any:
663670
return numeric
664671

665672

673+
def _resolve_count_series(series: Any, field: str, null_handling: Any) -> Any:
674+
"""Preserve values for count-like aggs, zero-filling nulls when requested."""
675+
if _get_null_mode(field, null_handling) == "zero":
676+
return series.fillna(0)
677+
return series
678+
679+
666680
def _extract_styler_formats(
667681
styler: Any,
668682
) -> tuple[dict[str, str], dict[str, str]]:

streamlit_pivot/frontend/src/engine/PivotData.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,6 +1897,18 @@ describe("PivotData - hybrid agg remap", () => {
18971897
expect(pd.getAggregator(["EU"], ["2023"]).value()).toBe(5);
18981898
});
18991899

1900+
it("count remap preserves zero for empty intersections", () => {
1901+
const countData: DataRecord[] = [
1902+
{ region: "US", year: "2023", revenue: 3 },
1903+
{ region: "EU", year: "2024", revenue: 5 },
1904+
];
1905+
const cfg = makeConfig({ aggregation: "count" });
1906+
const pd = new PivotData(countData, cfg, {
1907+
hybridAggRemap: { revenue: "sum" },
1908+
});
1909+
expect(pd.getAggregator(["US"], ["2024"]).value()).toBe(0);
1910+
});
1911+
19001912
it("count_distinct leaf cells with remap show pre-computed value", () => {
19011913
const data: DataRecord[] = [
19021914
{ region: "US", year: "2023", revenue: 7 },

streamlit_pivot/frontend/src/engine/PivotData.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,12 @@ export class PivotData {
429429
);
430430
}
431431

432+
private _emptyAggregatorForField(valField: string): Aggregator {
433+
return getAggregatorFactory(
434+
getAggregationForField(valField, this._config),
435+
).create();
436+
}
437+
432438
// ---------------------------------------------------------------------------
433439
// Filtering
434440
// ---------------------------------------------------------------------------
@@ -1124,7 +1130,7 @@ export class PivotData {
11241130
const keyStr = `${makeKeyString(rowKey)}\x01${makeKeyString(colKey)}\x01${field}`;
11251131
const agg = this._cellAggs.get(keyStr);
11261132
if (!agg) {
1127-
return this._factoryForField(field).create();
1133+
return this._emptyAggregatorForField(field);
11281134
}
11291135
return agg;
11301136
}
@@ -1145,7 +1151,7 @@ export class PivotData {
11451151
}
11461152
const agg = this._rowTotalAggs.get(keyStr);
11471153
if (!agg) {
1148-
return this._factoryForField(field).create();
1154+
return this._emptyAggregatorForField(field);
11491155
}
11501156
return agg;
11511157
}
@@ -1166,7 +1172,7 @@ export class PivotData {
11661172
}
11671173
const agg = this._colTotalAggs.get(keyStr);
11681174
if (!agg) {
1169-
return this._factoryForField(field).create();
1175+
return this._emptyAggregatorForField(field);
11701176
}
11711177
return agg;
11721178
}
@@ -1187,7 +1193,7 @@ export class PivotData {
11871193
}
11881194
const agg = this._grandTotalAggs.get(field);
11891195
if (!agg) {
1190-
return this._factoryForField(field).create();
1196+
return this._emptyAggregatorForField(field);
11911197
}
11921198
return agg;
11931199
}

streamlit_pivot/frontend/src/engine/aggregators.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,14 @@ describe("CountAggregator", () => {
119119
expect(agg.value()).toBe(3);
120120
});
121121

122+
it("ignores NaN values", () => {
123+
const agg = createAggregator("count");
124+
agg.push(1);
125+
agg.push(Number.NaN);
126+
agg.push(0);
127+
expect(agg.value()).toBe(2);
128+
});
129+
122130
it("returns 0 for all nulls", () => {
123131
const agg = createAggregator("count");
124132
agg.push(null);
@@ -249,6 +257,12 @@ describe("CountDistinctAggregator", () => {
249257
expect(agg.value()).toBe(3);
250258
});
251259

260+
it("ignores NaN values", () => {
261+
const agg = createAggregator("count_distinct");
262+
[1, Number.NaN, 1, 2].forEach((v) => agg.push(v));
263+
expect(agg.value()).toBe(2);
264+
});
265+
252266
it("returns null for empty input", () => {
253267
const agg = createAggregator("count_distinct");
254268
expect(agg.value()).toBeNull();

0 commit comments

Comments
 (0)