Skip to content

Commit 8262fdf

Browse files
mayagbarnesmayagbarnes
committed
More fixes
1 parent ef08d9b commit 8262fdf

5 files changed

Lines changed: 162 additions & 21 deletions

File tree

‎e2e_playwright/pivot_table_interactions_test.py‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,20 @@ def click_and_wait_for_count(locator, target, count: int, timeout: int = 10000)
6666
raise
6767

6868

69+
def click_and_wait_for_visible(locator, target, timeout: int = 10000) -> None:
70+
"""Click a control and retry once if the target does not become visible."""
71+
for attempt in range(2):
72+
locator.evaluate(
73+
"el => { el.scrollIntoView({ block: 'center', inline: 'nearest' }); el.click(); }"
74+
)
75+
try:
76+
expect(target).to_be_visible(timeout=timeout)
77+
return
78+
except AssertionError:
79+
if attempt == 1:
80+
raise
81+
82+
6983
def open_header_menu(page: Page, trigger_locator, menu_test_id: str):
7084
"""Open a header menu and wait for it to become visible."""
7185
expect(trigger_locator).to_be_visible(timeout=5000)
@@ -834,8 +848,10 @@ def test_readonly_mode_hides_toolbar_and_menu_actions(page_at_app: Page):
834848
expect(container.get_by_test_id("pivot-toolbar")).to_have_count(0)
835849
expect(container.get_by_test_id("header-menu-trigger-Region")).to_have_count(0)
836850

837-
container.get_by_test_id("pivot-data-cell").first.click()
838-
expect(container.get_by_test_id("drilldown-panel")).to_be_visible(timeout=5000)
851+
click_and_wait_for_visible(
852+
container.get_by_test_id("pivot-data-cell").first,
853+
page.get_by_test_id("drilldown-panel"),
854+
)
839855

840856

841857
def test_drilldown_disabled_no_panel(page_at_app: Page):

‎streamlit_pivot/__init__.py‎

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ def _prepare_threshold_hybrid_frame(
411411
agg = aggregation.get(vf, "sum")
412412
ser = filtered_df[vf]
413413
if agg in _NUMERIC_COERCE_AGGS:
414-
ser = pd.to_numeric(ser, errors="coerce")
414+
ser = _coerce_measure_series(ser, vf, null_handling)
415415
if agg == "avg":
416416
cnt = int(ser.count())
417417
row[vf] = float(ser.sum() / cnt) if cnt else float("nan")
@@ -451,7 +451,7 @@ def _prepare_threshold_hybrid_frame(
451451
col_type = column_types.get(dim) if column_types else None
452452
working[dim] = _resolve_dim_value_series(working[dim], col_type, mode, grain)
453453
for vf in numeric_coerce_fields:
454-
working[vf] = pd.to_numeric(working[vf], errors="coerce")
454+
working[vf] = _coerce_measure_series(working[vf], vf, null_handling)
455455

456456
out = (
457457
working.groupby(group_fields, dropna=False, observed=True, sort=False)
@@ -655,6 +655,14 @@ def _resolve_dim_value_series(
655655
return series.fillna("").astype(str)
656656

657657

658+
def _coerce_measure_series(series: Any, field: str, null_handling: Any) -> Any:
659+
"""Coerce measure values and zero-fill nulls when requested."""
660+
numeric = pd.to_numeric(series, errors="coerce")
661+
if _get_null_mode(field, null_handling) == "zero":
662+
return numeric.fillna(0)
663+
return numeric
664+
665+
658666
def _extract_styler_formats(
659667
styler: Any,
660668
) -> tuple[dict[str, str], dict[str, str]]:
@@ -942,15 +950,26 @@ def _build_sidecar_fingerprint(
942950
return json.dumps(obj, sort_keys=True, separators=(",", ":"))
943951

944952

945-
def _sidecar_agg_func(agg: str, series: Any) -> Any:
953+
def _sidecar_agg_func(
954+
agg: str,
955+
series: Any,
956+
field: str | None = None,
957+
null_handling: Any = None,
958+
) -> Any:
946959
"""Compute a single aggregate on a pandas Series for sidecar totals.
947960
948961
Applies pd.to_numeric coercion for numeric aggs to match the frontend's
949962
toNumber() semantics (non-numeric strings are dropped).
950963
"""
951964
if agg == "count_distinct":
965+
if field is not None and _get_null_mode(field, null_handling) == "zero":
966+
return series.fillna(0).nunique()
952967
return series.nunique()
953-
numeric = pd.to_numeric(series, errors="coerce")
968+
numeric = (
969+
_coerce_measure_series(series, field, null_handling)
970+
if field is not None
971+
else pd.to_numeric(series, errors="coerce")
972+
)
954973
if agg == "avg":
955974
return numeric.mean()
956975
if agg == "median":
@@ -970,6 +989,7 @@ def _sidecar_groupby_agg(
970989
df: Any,
971990
group_cols: list[str],
972991
sidecar_fields: dict[str, str],
992+
null_handling: Any = None,
973993
) -> list[dict[str, Any]]:
974994
"""GroupBy + aggregate for sidecar total entries."""
975995
if not group_cols or not sidecar_fields:
@@ -981,7 +1001,7 @@ def _sidecar_groupby_agg(
9811001
key = [str(k) for k in key]
9821002
values: dict[str, int | float | None] = {}
9831003
for field, agg in sidecar_fields.items():
984-
val = _sidecar_agg_func(agg, group_df[field])
1004+
val = _sidecar_agg_func(agg, group_df[field], field, null_handling)
9851005
values[field] = _normalize_sidecar_value(val)
9861006
entries.append({"key": key, "values": values})
9871007
return entries
@@ -1062,12 +1082,18 @@ def _compute_hybrid_totals(
10621082

10631083
grand: dict[str, int | float | None] = {}
10641084
for field, agg in sidecar_fields.items():
1065-
val = _sidecar_agg_func(agg, working[field])
1085+
val = _sidecar_agg_func(agg, working[field], field, null_handling)
10661086
grand[field] = _normalize_sidecar_value(val)
10671087

1068-
row_entries = _sidecar_groupby_agg(working, rows, sidecar_fields) if rows else []
1088+
row_entries = (
1089+
_sidecar_groupby_agg(working, rows, sidecar_fields, null_handling)
1090+
if rows
1091+
else []
1092+
)
10691093
col_entries = (
1070-
_sidecar_groupby_agg(working, columns, sidecar_fields) if columns else []
1094+
_sidecar_groupby_agg(working, columns, sidecar_fields, null_handling)
1095+
if columns
1096+
else []
10711097
)
10721098

10731099
result: dict[str, Any] = {
@@ -1093,7 +1119,7 @@ def _compute_hybrid_totals(
10931119
cp_key = parts[len(rows) :]
10941120
vals: dict[str, int | float | None] = {}
10951121
for field, agg in sidecar_fields.items():
1096-
v = _sidecar_agg_func(agg, group_df[field])
1122+
v = _sidecar_agg_func(agg, group_df[field], field, null_handling)
10971123
vals[field] = _normalize_sidecar_value(v)
10981124
col_prefix_entries.append(
10991125
{"key": cp_key, "row": row_key, "values": vals}
@@ -1110,7 +1136,7 @@ def _compute_hybrid_totals(
11101136
)
11111137
vals_grand: dict[str, int | float | None] = {}
11121138
for field, agg in sidecar_fields.items():
1113-
v = _sidecar_agg_func(agg, group_df[field])
1139+
v = _sidecar_agg_func(agg, group_df[field], field, null_handling)
11141140
vals_grand[field] = _normalize_sidecar_value(v)
11151141
col_prefix_grand_entries.append({"key": cp_key, "values": vals_grand})
11161142

@@ -1169,7 +1195,9 @@ def _compute_hybrid_totals(
11691195
)
11701196
vals_tp: dict[str, int | float | None] = {}
11711197
for field, agg in sidecar_fields.items():
1172-
v = _sidecar_agg_func(agg, group_df[field])
1198+
v = _sidecar_agg_func(
1199+
agg, group_df[field], field, null_handling
1200+
)
11731201
vals_tp[field] = _normalize_sidecar_value(v)
11741202
temporal_parent_entries.append(
11751203
{
@@ -1195,7 +1223,9 @@ def _compute_hybrid_totals(
11951223
)
11961224
vals_tp_g: dict[str, int | float | None] = {}
11971225
for field, agg in sidecar_fields.items():
1198-
v = _sidecar_agg_func(agg, group_df[field])
1226+
v = _sidecar_agg_func(
1227+
agg, group_df[field], field, null_handling
1228+
)
11991229
vals_tp_g[field] = _normalize_sidecar_value(v)
12001230
temporal_parent_grand_entries.append(
12011231
{
@@ -1262,7 +1292,9 @@ def _append_temporal_row_parent_entries(
12621292
col_key = col_key_builder(parts[len(rows) :])
12631293
vals_trp: dict[str, int | float | None] = {}
12641294
for field, agg in sidecar_fields.items():
1265-
v = _sidecar_agg_func(agg, group_df[field])
1295+
v = _sidecar_agg_func(
1296+
agg, group_df[field], field, null_handling
1297+
)
12661298
vals_trp[field] = _normalize_sidecar_value(v)
12671299
temporal_row_parent_entries.append(
12681300
{
@@ -1352,7 +1384,9 @@ def _append_temporal_row_parent_entries(
13521384
)
13531385
vals_trp_g: dict[str, int | float | None] = {}
13541386
for field, agg in sidecar_fields.items():
1355-
v = _sidecar_agg_func(agg, group_df[field])
1387+
v = _sidecar_agg_func(
1388+
agg, group_df[field], field, null_handling
1389+
)
13561390
vals_trp_g[field] = _normalize_sidecar_value(v)
13571391
temporal_row_parent_grand_entries.append(
13581392
{
@@ -1390,7 +1424,9 @@ def _append_temporal_row_parent_entries(
13901424
ck = parts[depth:]
13911425
vals_sub: dict[str, int | float | None] = {}
13921426
for field, agg in sidecar_fields.items():
1393-
v = _sidecar_agg_func(agg, group_df[field])
1427+
v = _sidecar_agg_func(
1428+
agg, group_df[field], field, null_handling
1429+
)
13941430
vals_sub[field] = _normalize_sidecar_value(v)
13951431
subtotal_entries.append({"key": rp, "col": ck, "values": vals_sub})
13961432

@@ -1402,7 +1438,7 @@ def _append_temporal_row_parent_entries(
14021438
parts = [str(p) for p in parts]
14031439
vals_rt: dict[str, int | float | None] = {}
14041440
for field, agg in sidecar_fields.items():
1405-
v = _sidecar_agg_func(agg, group_df[field])
1441+
v = _sidecar_agg_func(agg, group_df[field], field, null_handling)
14061442
vals_rt[field] = _normalize_sidecar_value(v)
14071443
subtotal_entries.append({"key": parts, "col": [], "values": vals_rt})
14081444

@@ -1420,7 +1456,9 @@ def _append_temporal_row_parent_entries(
14201456
cp_c = parts_c[depth:]
14211457
vals_cross: dict[str, int | float | None] = {}
14221458
for field, agg in sidecar_fields.items():
1423-
v = _sidecar_agg_func(agg, group_df[field])
1459+
v = _sidecar_agg_func(
1460+
agg, group_df[field], field, null_handling
1461+
)
14241462
vals_cross[field] = _normalize_sidecar_value(v)
14251463
cross_subtotal_entries.append(
14261464
{"key": rp_c, "col_prefix": cp_c, "values": vals_cross}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,24 @@ describe("PivotData - null handling", () => {
590590
expect(pd.getAggregator(["EU"], ["2024"]).count()).toBe(2);
591591
});
592592

593+
it("zero mode treats NaN values as 0 for averages", () => {
594+
const pd = new PivotData(
595+
[
596+
{ region: "EU", year: "2024", revenue: Number.NaN },
597+
{ region: "EU", year: "2024", revenue: 200 },
598+
],
599+
makeConfig({
600+
rows: ["region"],
601+
columns: ["year"],
602+
values: ["revenue"],
603+
aggregation: { revenue: "avg" },
604+
}),
605+
{ nullHandling: "zero" },
606+
);
607+
expect(pd.getAggregator(["EU"], ["2024"]).value()).toBe(100);
608+
expect(pd.getAggregator(["EU"], ["2024"]).count()).toBe(2);
609+
});
610+
593611
it("separate mode creates (null) bucket for dimension values", () => {
594612
const pd = new PivotData(
595613
DATA_WITH_NULLS,
@@ -1756,6 +1774,41 @@ describe("PivotData - hybrid sidecar override", () => {
17561774
expect(pd.getGrandTotal("revenue").value()).toBe(42.5);
17571775
});
17581776

1777+
it("accepts Python-style sidecar fingerprints with empty date_grains objects", () => {
1778+
const cfg = makeConfig({ aggregation: { revenue: "avg" } });
1779+
const hybridRows = [
1780+
{ region: "East", year: "2024", revenue: 5 },
1781+
{ region: "", year: "2024", revenue: 5 },
1782+
{ region: "West", year: "2024", revenue: 7 },
1783+
];
1784+
const totals: HybridTotals = {
1785+
sidecar_fingerprint: JSON.stringify({
1786+
adaptive_date_grains: {},
1787+
aggregation: { revenue: "avg" },
1788+
auto_date_hierarchy: true,
1789+
columns: ["year"],
1790+
date_grains: {},
1791+
filters: {},
1792+
null_handling: "zero",
1793+
rows: ["region"],
1794+
show_subtotals: false,
1795+
values: ["revenue"],
1796+
}),
1797+
grand: { revenue: 5.5 },
1798+
row: [
1799+
{ key: ["East"], values: { revenue: 5 } },
1800+
{ key: [""], values: { revenue: 5 } },
1801+
{ key: ["West"], values: { revenue: 7 } },
1802+
],
1803+
col: [{ key: ["2024"], values: { revenue: 5.5 } }],
1804+
};
1805+
const pd = new PivotData(hybridRows, cfg, {
1806+
hybridTotals: totals,
1807+
nullHandling: "zero",
1808+
});
1809+
expect(pd.getGrandTotal("revenue").value()).toBe(5.5);
1810+
});
1811+
17591812
it("getRowTotal uses sidecar value when available", () => {
17601813
const cfg = makeConfig({ aggregation: "median" });
17611814
const totals = makeHybridTotals(cfg, { revenue: 42.5 }, [

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ export function buildSidecarFingerprint(
132132
a.localeCompare(b),
133133
),
134134
)
135-
: undefined,
135+
: {},
136136
filters: Object.fromEntries(
137137
Object.entries(filters)
138138
.sort(([a], [b]) => a.localeCompare(b))
@@ -559,7 +559,7 @@ export class PivotData {
559559
}
560560

561561
private _resolveAggValue(field: string, raw: unknown): unknown {
562-
if (raw == null) {
562+
if (raw == null || (typeof raw === "number" && Number.isNaN(raw))) {
563563
const mode = getNullMode(field, this._options.nullHandling);
564564
return mode === "zero" ? 0 : raw;
565565
}

‎tests/test_threshold_hybrid.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -979,6 +979,26 @@ def test_prepare_hybrid_frame_separate_mode_null_filter(pivot_module):
979979
assert float(result["revenue"].values[0]) == 600.0
980980

981981

982+
def test_prepare_threshold_hybrid_frame_avg_zero_treats_null_as_zero(pivot_module):
983+
df = pd.DataFrame(
984+
{
985+
"region": ["EU", "EU"],
986+
"year": ["2024", "2024"],
987+
"revenue": [None, 200.0],
988+
}
989+
)
990+
cfg = {
991+
"version": pivot_module.CONFIG_SCHEMA_VERSION,
992+
"rows": ["region"],
993+
"columns": ["year"],
994+
"values": ["revenue"],
995+
"aggregation": {"revenue": "avg"},
996+
"synthetic_measures": [],
997+
}
998+
result = pivot_module._prepare_threshold_hybrid_frame(df, cfg, null_handling="zero")
999+
assert float(result["revenue"].iloc[0]) == 100.0
1000+
1001+
9821002
def test_prepare_hybrid_frame_no_filters_unchanged(pivot_module):
9831003
"""No filters produces same output as before."""
9841004
df = pd.DataFrame(
@@ -1075,6 +1095,20 @@ def test_includes_avg(self, pivot_module, df):
10751095
assert sidecar is not None
10761096
assert abs(sidecar["grand"]["revenue"] - df["revenue"].mean()) < 1e-9
10771097

1098+
def test_zero_null_handling_affects_avg(self, pivot_module):
1099+
df = pd.DataFrame(
1100+
{
1101+
"region": ["EU", "EU"],
1102+
"year": ["2024", "2024"],
1103+
"revenue": [None, 200.0],
1104+
}
1105+
)
1106+
sidecar = pivot_module._compute_hybrid_totals(df, self._cfg("avg"), "zero")
1107+
assert sidecar is not None
1108+
assert sidecar["grand"]["revenue"] == 100.0
1109+
row_vals = {tuple(e["key"]): e["values"]["revenue"] for e in sidecar["row"]}
1110+
assert row_vals[("EU",)] == 100.0
1111+
10781112
def test_respects_filters(self, pivot_module, df):
10791113
cfg = self._cfg("median", filters={"region": {"include": ["US"]}})
10801114
sidecar = pivot_module._compute_hybrid_totals(df, cfg, None)

0 commit comments

Comments
 (0)