From 3e08ebb96bd950e7100a81236f9291dd1941a7bb Mon Sep 17 00:00:00 2001 From: Chris Nicholas <4948774+cnicholas@users.noreply.github.com> Date: Sat, 17 Oct 2026 13:36:44 -0400 Subject: [PATCH] Keep the subgroup label and n on every Xbar/S chart table (#143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **What:** Xbar/S `chart_table` shows `subgroup` and `n` whatever the `by=`; Xbar/S chart data carries `n`. - **Why:** with a `by=` other than time alone (pooled subsets, all factors, factors without time, one factor) the table dropped both columns. A row couldn't be traced to its subgroup and stepped limits went unexplained. The app shows these tables directly. - **Scope:** the Xbar/S aggregation already counts each subgroup; it now keeps that `n` in the chart data instead of dropping it, and names its label column in metadata (`group_col`). The table reads both, and the per-cell re-join that could never size a pooled subgroup (`_join_n_on_composite_group`) is gone. - [x] **Default behavior unchanged:** limits, centres and statistics byte-identical on 104 Xbar/S requests over T100 PM SDS 1 and 3 (snapshot vs main) - [x] **Residuals unaffected:** no residual or design-state change - [x] **Row/index alignment preserved:** row count and order unchanged - [x] **Lane boundaries preserved:** untouched - [ ] **Output schema compatible:** additive — Xbar/S chart data gains `n`; metadata gains `group_col` (CHANGELOG "Changed") - **New semantics:** - `get_chart('Xbar'/'S')` has `n` after the subgroup label (after `stratify_col`, `group_col` on stratified charts); Excel chart sheets too - Every Xbar/S `chart_table` has `subgroup, n`; a pooled subgroup's n covers all its cells - `n` is the count the limits rest on. On residual charts whose limits come from R2 (R1, R3-R5) in a design with singletons, the stream's first cell now shows one fewer than its cell size, as its S and limits use (main showed the cell size there, which disagreed with its own limits; one row on PM SDS 3) - **n source:** the aggregation's own count (Chris's call), not a re-join from per-cell sizes - **Label column:** declared by the chart (`metadata['group_col']`) rather than guessed from column names; stratified and X/mR tables keep the `rsg` fallback - **NaN policy:** n counts the limits column's non-missing values, as the aggregation always has - **n_mode='average':** `n` stays the actual sizes; the average remains in `N` / `metadata['n_avg']` - **P0 risks:** a limits change hiding behind the schema change - **Mitigations:** snapshot comparison against main (limits/stats identical, only `n` added); golden masters regenerated only after checking the diff was exactly the `n` column and `group_col` key - [x] TestXbarSSubgroupAndN (test_result_tabular.py) — 23 cases on T100: every by= variant has subgroup and an independently counted n (Xbar n >= 1, S n >= 2); solo S == companion S; default and stratified tables equal cell sizes; R5 n follows R2's count; n_mode='average' keeps actual sizes; n sits after the label in chart data - [x] Golden masters for paired_xbar_s, residual_chart, stratified_balanced, stratified_uneven regenerated (n column + group_col only) - [x] 16 of the new cases fail on main's code; the 7 regression guards pass on both - [x] `pytest tests/` — 2567 passed, 7 skipped - [x] `python validation/e2e_bishop_report.py` — 1547 passed, 0 failed, 4 pending (14 runs); docs/reference/validation.md unchanged - [x] App fast tests against the local library (PYTHONPATH) — 193 passed - [x] Survey of by= variants on make_design(1): every Xbar/S table shows subgroup and n - **Docs:** chart_table docstring, api.md, CHANGELOG Fixed + Changed; stored outputs of the cells printing Xbar/S chart data refreshed (xbar-s-analysis cells 11, 12, 24, 25; quickstart cell 13) - **Follow-ups (not in this PR):** none --- CHANGELOG.md | 13 +++ docs/getting-started/quickstart.ipynb | 33 ++++-- docs/reference/api.md | 5 + docs/tutorials/xbar-s-analysis.ipynb | 102 ++++++++++++------ processbehavior/analysis.py | 13 ++- processbehavior/analysis_result.py | 9 +- processbehavior/result_tabular.py | 70 ++++-------- processbehavior/types.py | 3 + .../paired_xbar_s/S_data.parquet | Bin 4841 -> 5373 bytes .../paired_xbar_s/S_metadata.json | 1 + .../paired_xbar_s/Xbar_data.parquet | Bin 4922 -> 5458 bytes .../paired_xbar_s/Xbar_metadata.json | 1 + .../residual_chart/S_data.parquet | Bin 4632 -> 5142 bytes .../residual_chart/S_metadata.json | 1 + .../stratified_balanced/S_data.parquet | Bin 4632 -> 5142 bytes .../stratified_balanced/S_metadata.json | 1 + .../stratified_balanced/Xbar_data.parquet | Bin 4687 -> 5201 bytes .../stratified_balanced/Xbar_metadata.json | 1 + .../stratified_uneven/S_data.parquet | Bin 4567 -> 5098 bytes .../stratified_uneven/S_metadata.json | 1 + .../stratified_uneven/Xbar_data.parquet | Bin 4630 -> 5165 bytes .../stratified_uneven/Xbar_metadata.json | 1 + tests/test_result_tabular.py | 98 ++++++++++++++++- 23 files changed, 251 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09a447e..5bd8d52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 whole column as one replicated cell: ADS 1, a recommended one-point Xbar chart, the "within-cell replication" series-length line, and an X chart plotted against the response values. Its chart tables now start with `Observation` (#129). +- **Xbar and S chart data carry `n`.** `get_chart('Xbar')` / `get_chart('S')`, and the Excel chart + sheets, gain an `n` column after the subgroup label. Unstratified Xbar/S metadata names the label + column as `group_col` (#143). + +### Fixed +- **Xbar/S `chart_table` keeps the subgroup label and `n` on every `by=`.** With a `by=` other than + time alone, such as a pooled subset (`by=['FACTOR 1', time]`), all factors, factors without time, + or one factor, the table dropped both columns, so a row couldn't be traced to its subgroup and + stepped limits went unexplained. `n` is now the aggregation's own count, which covers every cell + a pooled subgroup spans. This also corrects one row on residual charts whose limits rest on R2 + (R1, R3, R4, R5) in a design with singletons: the stream's first cell, where R2 is undefined at + the first value, showed its cell size, not the one-fewer values its S and limits use. Limits, + centres and statistics are unchanged (#143). ## [0.4.0] - 2026-10-14 diff --git a/docs/getting-started/quickstart.ipynb b/docs/getting-started/quickstart.ipynb index d82b9bf..22d12a1 100644 --- a/docs/getting-started/quickstart.ipynb +++ b/docs/getting-started/quickstart.ipynb @@ -478,6 +478,7 @@ " \n", " \n", " group\n", + " n\n", " xbar\n", " center\n", " lpl\n", @@ -489,6 +490,7 @@ " \n", " 0\n", " A_1\n", + " 3\n", " 100.670\n", " 100.292\n", " 96.887\n", @@ -498,6 +500,7 @@ " \n", " 1\n", " A_2\n", + " 3\n", " 97.733\n", " 100.292\n", " 96.887\n", @@ -507,6 +510,7 @@ " \n", " 2\n", " A_3\n", + " 3\n", " 98.943\n", " 100.292\n", " 96.887\n", @@ -516,6 +520,7 @@ " \n", " 3\n", " A_4\n", + " 3\n", " 97.947\n", " 100.292\n", " 96.887\n", @@ -525,6 +530,7 @@ " \n", " 4\n", " A_5\n", + " 3\n", " 99.270\n", " 100.292\n", " 96.887\n", @@ -534,6 +540,7 @@ " \n", " 5\n", " A_6\n", + " 3\n", " 98.820\n", " 100.292\n", " 96.887\n", @@ -543,6 +550,7 @@ " \n", " 6\n", " A_7\n", + " 3\n", " 99.273\n", " 100.292\n", " 96.887\n", @@ -552,6 +560,7 @@ " \n", " 7\n", " A_8\n", + " 3\n", " 99.737\n", " 100.292\n", " 96.887\n", @@ -561,6 +570,7 @@ " \n", " 8\n", " A_9\n", + " 3\n", " 100.373\n", " 100.292\n", " 96.887\n", @@ -570,6 +580,7 @@ " \n", " 9\n", " A_10\n", + " 3\n", " 98.843\n", " 100.292\n", " 96.887\n", @@ -581,17 +592,17 @@ "" ], "text/plain": [ - " group xbar center lpl upl beyond_limits\n", - "0 A_1 100.670 100.292 96.887 103.696 0\n", - "1 A_2 97.733 100.292 96.887 103.696 0\n", - "2 A_3 98.943 100.292 96.887 103.696 0\n", - "3 A_4 97.947 100.292 96.887 103.696 0\n", - "4 A_5 99.270 100.292 96.887 103.696 0\n", - "5 A_6 98.820 100.292 96.887 103.696 0\n", - "6 A_7 99.273 100.292 96.887 103.696 0\n", - "7 A_8 99.737 100.292 96.887 103.696 0\n", - "8 A_9 100.373 100.292 96.887 103.696 0\n", - "9 A_10 98.843 100.292 96.887 103.696 0" + " group n xbar center lpl upl beyond_limits\n", + "0 A_1 3 100.670 100.292 96.887 103.696 0\n", + "1 A_2 3 97.733 100.292 96.887 103.696 0\n", + "2 A_3 3 98.943 100.292 96.887 103.696 0\n", + "3 A_4 3 97.947 100.292 96.887 103.696 0\n", + "4 A_5 3 99.270 100.292 96.887 103.696 0\n", + "5 A_6 3 98.820 100.292 96.887 103.696 0\n", + "6 A_7 3 99.273 100.292 96.887 103.696 0\n", + "7 A_8 3 99.737 100.292 96.887 103.696 0\n", + "8 A_9 3 100.373 100.292 96.887 103.696 0\n", + "9 A_10 3 98.843 100.292 96.887 103.696 0" ] }, "execution_count": 6, diff --git a/docs/reference/api.md b/docs/reference/api.md index 15ea605..78456c3 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -253,6 +253,11 @@ result.charts['Xbar']['metadata'] # dict result.charts['Xbar']['strata'] # per-stratum payloads, when stratified ``` +Xbar and S chart data carry `n`, each subgroup's own count of the values its limits +rest on, right after the subgroup label. `chart_table` shows it, with the label as +`subgroup`, whatever the `by=`: a pooled subgroup (`by=['lane', 'hour']`) counts all +the cells it pools. + ### Statistics — the four-key contract ```python diff --git a/docs/tutorials/xbar-s-analysis.ipynb b/docs/tutorials/xbar-s-analysis.ipynb index 9e1272e..4ac5e6f 100644 --- a/docs/tutorials/xbar-s-analysis.ipynb +++ b/docs/tutorials/xbar-s-analysis.ipynb @@ -397,6 +397,7 @@ " \n", " \n", " group\n", + " n\n", " xbar\n", " center\n", " lpl\n", @@ -408,6 +409,7 @@ " \n", " 0\n", " A_1\n", + " 4\n", " 100.948\n", " 101.549\n", " 99.448\n", @@ -417,6 +419,7 @@ " \n", " 1\n", " A_2\n", + " 4\n", " 98.815\n", " 101.549\n", " 99.448\n", @@ -426,6 +429,7 @@ " \n", " 2\n", " A_3\n", + " 4\n", " 100.145\n", " 101.549\n", " 99.448\n", @@ -435,6 +439,7 @@ " \n", " 3\n", " A_4\n", + " 4\n", " 99.820\n", " 101.549\n", " 99.448\n", @@ -444,6 +449,7 @@ " \n", " 4\n", " A_5\n", + " 4\n", " 100.648\n", " 101.549\n", " 99.448\n", @@ -453,6 +459,7 @@ " \n", " 5\n", " A_6\n", + " 4\n", " 100.388\n", " 101.549\n", " 99.448\n", @@ -462,6 +469,7 @@ " \n", " 6\n", " A_7\n", + " 4\n", " 101.700\n", " 101.549\n", " 99.448\n", @@ -471,6 +479,7 @@ " \n", " 7\n", " A_8\n", + " 4\n", " 102.075\n", " 101.549\n", " 99.448\n", @@ -480,6 +489,7 @@ " \n", " 8\n", " B_1\n", + " 4\n", " 102.705\n", " 101.549\n", " 99.448\n", @@ -489,6 +499,7 @@ " \n", " 9\n", " B_2\n", + " 4\n", " 101.168\n", " 101.549\n", " 99.448\n", @@ -500,17 +511,17 @@ "" ], "text/plain": [ - " group xbar center lpl upl beyond_limits\n", - "0 A_1 100.948 101.549 99.448 103.651 0\n", - "1 A_2 98.815 101.549 99.448 103.651 -1\n", - "2 A_3 100.145 101.549 99.448 103.651 0\n", - "3 A_4 99.820 101.549 99.448 103.651 0\n", - "4 A_5 100.648 101.549 99.448 103.651 0\n", - "5 A_6 100.388 101.549 99.448 103.651 0\n", - "6 A_7 101.700 101.549 99.448 103.651 0\n", - "7 A_8 102.075 101.549 99.448 103.651 0\n", - "8 B_1 102.705 101.549 99.448 103.651 0\n", - "9 B_2 101.168 101.549 99.448 103.651 0" + " group n xbar center lpl upl beyond_limits\n", + "0 A_1 4 100.948 101.549 99.448 103.651 0\n", + "1 A_2 4 98.815 101.549 99.448 103.651 -1\n", + "2 A_3 4 100.145 101.549 99.448 103.651 0\n", + "3 A_4 4 99.820 101.549 99.448 103.651 0\n", + "4 A_5 4 100.648 101.549 99.448 103.651 0\n", + "5 A_6 4 100.388 101.549 99.448 103.651 0\n", + "6 A_7 4 101.700 101.549 99.448 103.651 0\n", + "7 A_8 4 102.075 101.549 99.448 103.651 0\n", + "8 B_1 4 102.705 101.549 99.448 103.651 0\n", + "9 B_2 4 101.168 101.549 99.448 103.651 0" ] }, "execution_count": 5, @@ -568,6 +579,7 @@ " \n", " \n", " group\n", + " n\n", " s\n", " center\n", " lpl\n", @@ -579,6 +591,7 @@ " \n", " 0\n", " A_1\n", + " 4\n", " 1.025\n", " 1.29\n", " 0.0\n", @@ -588,6 +601,7 @@ " \n", " 1\n", " A_2\n", + " 4\n", " 1.523\n", " 1.29\n", " 0.0\n", @@ -597,6 +611,7 @@ " \n", " 2\n", " A_3\n", + " 4\n", " 1.030\n", " 1.29\n", " 0.0\n", @@ -606,6 +621,7 @@ " \n", " 3\n", " A_4\n", + " 4\n", " 1.646\n", " 1.29\n", " 0.0\n", @@ -615,6 +631,7 @@ " \n", " 4\n", " A_5\n", + " 4\n", " 1.483\n", " 1.29\n", " 0.0\n", @@ -624,6 +641,7 @@ " \n", " 5\n", " A_6\n", + " 4\n", " 0.732\n", " 1.29\n", " 0.0\n", @@ -633,6 +651,7 @@ " \n", " 6\n", " A_7\n", + " 4\n", " 2.735\n", " 1.29\n", " 0.0\n", @@ -642,6 +661,7 @@ " \n", " 7\n", " A_8\n", + " 4\n", " 1.175\n", " 1.29\n", " 0.0\n", @@ -651,6 +671,7 @@ " \n", " 8\n", " B_1\n", + " 4\n", " 1.316\n", " 1.29\n", " 0.0\n", @@ -660,6 +681,7 @@ " \n", " 9\n", " B_2\n", + " 4\n", " 1.117\n", " 1.29\n", " 0.0\n", @@ -671,17 +693,17 @@ "" ], "text/plain": [ - " group s center lpl upl beyond_limits\n", - "0 A_1 1.025 1.29 0.0 2.924 0\n", - "1 A_2 1.523 1.29 0.0 2.924 0\n", - "2 A_3 1.030 1.29 0.0 2.924 0\n", - "3 A_4 1.646 1.29 0.0 2.924 0\n", - "4 A_5 1.483 1.29 0.0 2.924 0\n", - "5 A_6 0.732 1.29 0.0 2.924 0\n", - "6 A_7 2.735 1.29 0.0 2.924 0\n", - "7 A_8 1.175 1.29 0.0 2.924 0\n", - "8 B_1 1.316 1.29 0.0 2.924 0\n", - "9 B_2 1.117 1.29 0.0 2.924 0" + " group n s center lpl upl beyond_limits\n", + "0 A_1 4 1.025 1.29 0.0 2.924 0\n", + "1 A_2 4 1.523 1.29 0.0 2.924 0\n", + "2 A_3 4 1.030 1.29 0.0 2.924 0\n", + "3 A_4 4 1.646 1.29 0.0 2.924 0\n", + "4 A_5 4 1.483 1.29 0.0 2.924 0\n", + "5 A_6 4 0.732 1.29 0.0 2.924 0\n", + "6 A_7 4 2.735 1.29 0.0 2.924 0\n", + "7 A_8 4 1.175 1.29 0.0 2.924 0\n", + "8 B_1 4 1.316 1.29 0.0 2.924 0\n", + "9 B_2 4 1.117 1.29 0.0 2.924 0" ] }, "execution_count": 6, @@ -3367,6 +3389,7 @@ " \n", " \n", " group\n", + " n\n", " s\n", " center\n", " lpl\n", @@ -3378,6 +3401,7 @@ " \n", " 0\n", " A_1\n", + " 4\n", " 1.025\n", " 1.29\n", " 0.0\n", @@ -3387,6 +3411,7 @@ " \n", " 1\n", " A_2\n", + " 4\n", " 1.523\n", " 1.29\n", " 0.0\n", @@ -3396,6 +3421,7 @@ " \n", " 2\n", " A_3\n", + " 4\n", " 1.030\n", " 1.29\n", " 0.0\n", @@ -3405,6 +3431,7 @@ " \n", " 3\n", " A_4\n", + " 4\n", " 1.646\n", " 1.29\n", " 0.0\n", @@ -3414,6 +3441,7 @@ " \n", " 4\n", " A_5\n", + " 4\n", " 1.483\n", " 1.29\n", " 0.0\n", @@ -3425,12 +3453,12 @@ "" ], "text/plain": [ - " group s center lpl upl beyond_limits\n", - "0 A_1 1.025 1.29 0.0 2.924 0\n", - "1 A_2 1.523 1.29 0.0 2.924 0\n", - "2 A_3 1.030 1.29 0.0 2.924 0\n", - "3 A_4 1.646 1.29 0.0 2.924 0\n", - "4 A_5 1.483 1.29 0.0 2.924 0" + " group n s center lpl upl beyond_limits\n", + "0 A_1 4 1.025 1.29 0.0 2.924 0\n", + "1 A_2 4 1.523 1.29 0.0 2.924 0\n", + "2 A_3 4 1.030 1.29 0.0 2.924 0\n", + "3 A_4 4 1.646 1.29 0.0 2.924 0\n", + "4 A_5 4 1.483 1.29 0.0 2.924 0" ] }, "execution_count": 13, @@ -3491,6 +3519,7 @@ " \n", " \n", " group\n", + " n\n", " s\n", " center\n", " lpl\n", @@ -3502,6 +3531,7 @@ " \n", " 0\n", " A_1\n", + " 4\n", " 1.025\n", " 1.29\n", " 0.0\n", @@ -3511,6 +3541,7 @@ " \n", " 1\n", " A_2\n", + " 4\n", " 1.523\n", " 1.29\n", " 0.0\n", @@ -3520,6 +3551,7 @@ " \n", " 2\n", " A_3\n", + " 4\n", " 1.030\n", " 1.29\n", " 0.0\n", @@ -3529,6 +3561,7 @@ " \n", " 3\n", " A_4\n", + " 4\n", " 1.646\n", " 1.29\n", " 0.0\n", @@ -3538,6 +3571,7 @@ " \n", " 4\n", " A_5\n", + " 4\n", " 1.483\n", " 1.29\n", " 0.0\n", @@ -3549,12 +3583,12 @@ "" ], "text/plain": [ - " group s center lpl upl beyond_limits\n", - "0 A_1 1.025 1.29 0.0 2.924 0\n", - "1 A_2 1.523 1.29 0.0 2.924 0\n", - "2 A_3 1.030 1.29 0.0 2.924 0\n", - "3 A_4 1.646 1.29 0.0 2.924 0\n", - "4 A_5 1.483 1.29 0.0 2.924 0" + " group n s center lpl upl beyond_limits\n", + "0 A_1 4 1.025 1.29 0.0 2.924 0\n", + "1 A_2 4 1.523 1.29 0.0 2.924 0\n", + "2 A_3 4 1.030 1.29 0.0 2.924 0\n", + "3 A_4 4 1.646 1.29 0.0 2.924 0\n", + "4 A_5 4 1.483 1.29 0.0 2.924 0" ] }, "execution_count": 14, diff --git a/processbehavior/analysis.py b/processbehavior/analysis.py index 8db5514..b04585e 100644 --- a/processbehavior/analysis.py +++ b/processbehavior/analysis.py @@ -1379,7 +1379,9 @@ def _calculate_xbar( # noqa: C901 else: group_col = None - cols_to_keep = [group_col, 'xbar', 'center', 'lpl', 'upl', 'beyond_limits'] + # n is each subgroup's own count from the aggregation above: the size its limits + # used. A pooled subgroup spans several cells, so no per-cell size could stand in (#143). + cols_to_keep = [group_col, 'n', 'xbar', 'center', 'lpl', 'upl', 'beyond_limits'] cols_to_keep = [c for c in cols_to_keep if c in xbar.columns] xbar = xbar[cols_to_keep] result['Xbar'] = { @@ -1392,6 +1394,7 @@ def _calculate_xbar( # noqa: C901 'n_sigma': self.request.n_sigma, 'n_mode': n_mode, **({'n_avg': round(n_avg, spec.round_to)} if n_avg is not None else {}), + **({'group_col': group_col} if group_col else {}), }, } @@ -1546,7 +1549,7 @@ def _calculate_xbar_stratified( # Select output columns group_col = groupby_cols[0] if groupby_cols else None - cols_to_keep = [stratify_col, group_col, 'xbar', 'center', 'lpl', 'upl', 'beyond_limits'] + cols_to_keep = [stratify_col, group_col, 'n', 'xbar', 'center', 'lpl', 'upl', 'beyond_limits'] cols_to_keep = [c for c in cols_to_keep if c is not None and c in chart_out.columns] chart_out = chart_out[cols_to_keep] @@ -1710,7 +1713,7 @@ def _calculate_s_stratified( published_strata = [s for s in all_strata if s not in insufficient_set] group_col = groupby_cols[0] if groupby_cols else None - cols_to_keep = [stratify_col, group_col, 's', 'center', 'lpl', 'upl', 'beyond_limits'] + cols_to_keep = [stratify_col, group_col, 'n', 's', 'center', 'lpl', 'upl', 'beyond_limits'] cols_to_keep = [c for c in cols_to_keep if c is not None and c in chart_out.columns] chart_out = chart_out[cols_to_keep] @@ -1887,7 +1890,8 @@ def _calculate_s( # noqa: C901 elif len(groupby_cols) > 1 and group_col == 'group' and 'group' not in out.columns: out['group'] = out[groupby_cols].astype(str).agg('_'.join, axis=1) - cols_to_keep = [group_col, 's', 'center', 'lpl', 'upl', 'beyond_limits'] + # n: each subgroup's own count, as on Xbar (#143). + cols_to_keep = [group_col, 'n', 's', 'center', 'lpl', 'upl', 'beyond_limits'] cols_to_keep = [c for c in cols_to_keep if c in out.columns] out = out[cols_to_keep] out = out.round(spec.round_to) @@ -1917,6 +1921,7 @@ def _calculate_s( # noqa: C901 'n_sigma': self.request.n_sigma, 'n_mode': n_mode, **({'n_avg': round(n_avg, spec.round_to)} if n_avg is not None else {}), + **({'group_col': group_col} if group_col else {}), }, } } diff --git a/processbehavior/analysis_result.py b/processbehavior/analysis_result.py index 4a86b39..d0d7267 100644 --- a/processbehavior/analysis_result.py +++ b/processbehavior/analysis_result.py @@ -808,8 +808,13 @@ def chart_table( ------- pd.DataFrame Summary table with columns: - - subgroup: Subgroup identifier (RSG) — when chart_data has rsg - - n: Number of observations in subgroup + - subgroup: Subgroup identifier — on every Xbar/S table, whatever + the ``by=`` (a pooled subgroup is labelled by its ``by`` values), + and on X/mR tables when the chart data has an rsg + - n: Number of values in the subgroup that its limits rest on — + the Xbar/S aggregation's own count, so a pooled subgroup's n + covers all its cells, and a residual whose first value is + undefined (R2) counts one fewer - value: The statistic value (xbar, s, etc.) - center: Center line value - lpl: Lower process limit diff --git a/processbehavior/result_tabular.py b/processbehavior/result_tabular.py index d9ed12c..44ac81d 100644 --- a/processbehavior/result_tabular.py +++ b/processbehavior/result_tabular.py @@ -16,7 +16,6 @@ import pandas as pd -from .data_preparation import encode_rsg_series from .exceptions import ChartNotAvailableError if TYPE_CHECKING: @@ -68,21 +67,23 @@ def build_chart_table( ) -> pd.DataFrame: """Build the summary DataFrame for one chart on `result`. - Identifies the value column, joins the per-cell `n` from the analysis - dataset if available, selects logical output columns, and optionally - converts `beyond_limits` into ↓/↑ symbols. + Identifies the value column, takes the subgroup size `n` from the chart + data (Xbar/S carry their own) or joins the per-cell `n` from the analysis + dataset, selects logical output columns, and optionally converts + `beyond_limits` into ↓/↑ symbols. Parameters mirror `AnalysisResult.chart_table` exactly. See that docstring for the column contract. """ chart = _resolve_chart(result, chart) chart_data = result.charts[chart]["data"].copy() + group_col = result.charts[chart]["metadata"].get("group_col") value_col = _resolve_value_col(result, chart_data) chart_data = _join_n_column(result, chart_data) output_cols, col_renames = _output_column_plan( - chart_data, value_col, include_signal_col, chart=chart + chart_data, value_col, include_signal_col, chart=chart, group_col=group_col ) table = chart_data[output_cols].copy().rename(columns=col_renames) @@ -173,7 +174,8 @@ def _join_n_column(result: AnalysisResult, chart_data: pd.DataFrame) -> pd.DataF """Merge per-cell subgroup size `n` from the analysis dataset. Returns `chart_data` unchanged when: - - `n` already in chart_data + - `n` already in chart_data — every Xbar/S chart carries its subgroups' + own counts, which a per-cell join could not give a pooled subgroup (#143) - no analysis dataset available - `n` not in analysis dataset - no matching kt columns (factor × time grain) @@ -199,12 +201,7 @@ def _join_n_column(result: AnalysisResult, chart_data: pd.DataFrame) -> pd.DataF if spec.has_time and spec.time_var in ads.columns and spec.time_var in chart_data.columns: kt_cols.append(spec.time_var) if not kt_cols: - # Xbar/S identify a subgroup with a single composite "group" column - # (rsg and time already joined, e.g. 'F1_1_F2_1_3') rather than the - # separate key columns X/mR carry. Rebuild the same composite on the - # ads side and join on that — otherwise `n` silently never lands on - # exactly the charts whose limits vary with it. - return _join_n_on_composite_group(ads, spec, chart_data) + return chart_data n_per_kt = ads.groupby(kt_cols, observed=True)["n"].first().reset_index() @@ -220,42 +217,12 @@ def _join_n_column(result: AnalysisResult, chart_data: pd.DataFrame) -> pd.DataF return chart_data -def _join_n_on_composite_group(ads: pd.DataFrame, spec, chart_data: pd.DataFrame) -> pd.DataFrame: - """Merge `n` onto a chart keyed by a single composite subgroup column. - - Xbar/S label each subgroup with the encoded (rsg, time) cell — the same - encoding :func:`encode_rsg_series` produces — so the join key is rebuilt - rather than matched column-by-column. Returns `chart_data` unchanged if - the composite cannot be formed or does not match. - """ - if "group" not in chart_data.columns: - return chart_data - - key_cols = [c for c in (spec.rsg_var_name, spec.time_var) if c and c in ads.columns] - if len(key_cols) < 2: - return chart_data - - n_per_cell = ads.groupby(key_cols, observed=True)["n"].first().reset_index() - n_per_cell["group"] = encode_rsg_series(n_per_cell, key_cols) - n_per_cell = n_per_cell[["group", "n"]] - - chart_data = chart_data.copy() - chart_data["group"] = chart_data["group"].astype(str) - - with contextlib.suppress(ValueError, TypeError): - merged = chart_data.merge(n_per_cell, on="group", how="left") - # Only adopt the merge if it actually matched; a delimiter collision - # would otherwise replace a missing column with a column of NaN. - if merged["n"].notna().any(): - return merged - return chart_data - - def _output_column_plan( chart_data: pd.DataFrame, value_col: str | None, include_signal_col: bool, chart: str | None = None, + group_col: str | None = None, ) -> tuple[list[str], dict[str, str]]: """Decide which columns to keep and how to rename them for display.""" output_cols: list[str] = [] @@ -268,12 +235,17 @@ def _output_column_plan( output_cols.append("obs_id") renames["obs_id"] = "Obs" - # The subgroup identifier is "rsg" on X/mR charts but "group" on Xbar/S. - # Only "rsg" was recognised, so chart_table('Xbar') silently dropped the - # subgroup column (and "n" with it) that its own docstring promises. - subgroup_col = next( - (col for col in ("rsg", "group") if col in chart_data.columns), None - ) + # The subgroup identifier: the column an unstratified Xbar/S chart declares in + # its metadata ("group" by default, "subgroup" for a multi-column by=, or the + # factor's own name for by=[factor]); otherwise "rsg" on X/mR and stratified + # charts. Recognising only "rsg"/"group" dropped the label on every by= Xbar/S + # table (#143). + if group_col is not None and group_col in chart_data.columns: + subgroup_col: str | None = group_col + else: + subgroup_col = next( + (col for col in ("rsg", "group", "subgroup") if col in chart_data.columns), None + ) if subgroup_col: output_cols.append(subgroup_col) renames[subgroup_col] = "subgroup" diff --git a/processbehavior/types.py b/processbehavior/types.py index ffaaeda..cc84d80 100644 --- a/processbehavior/types.py +++ b/processbehavior/types.py @@ -93,6 +93,8 @@ class ChartMetadata(TypedDict, total=False): - ``chart_type`` / ``value_col`` / ``center_col``: every chart. - ``stratified`` / ``stratify_col`` / ``stratify_by``: stratified charts only. + - ``group_col``: unstratified Xbar/S — the chart-data column that labels + each subgroup (``group``, ``subgroup``, or the factor's own name). - ``lane_boundaries``: stratified X/mR charts with collapsed factors. - ``insufficient_strata``: mR/r charts that dropped first-per-stratum. - ``phased`` / ``single_point_phases``: charts run with @@ -109,6 +111,7 @@ class ChartMetadata(TypedDict, total=False): stratified: bool stratify_col: str | None stratify_by: list[str] + group_col: str # Unstratified Xbar/S: the subgroup label column lane_boundaries: dict[str, list[float]] | None insufficient_strata: list[str] | None phased: bool diff --git a/tests/fixtures/golden_masters/paired_xbar_s/S_data.parquet b/tests/fixtures/golden_masters/paired_xbar_s/S_data.parquet index b35badcd86b8093ad3998c621fc4864c6782e348..a7b723793a1cc360c6d30a1d7403fa078996856b 100644 GIT binary patch delta 819 zcmb7CPfrs;6rb7dZWl^x+qmmCRwUIfkkl=I+Jq8&m|ZGGflv_C#6-Jo-J*1DY%0`{ z)DM8s@ID0w6JswNd*tY6ARg2M@#IMpXSUFbUVNE(e}D6PZ+7Qf{P+0BQT%X1))!?_ zMqz|h-h&44O%C;hKeIBDughZzIWGq(P>ib5c}11Zu?iadM;4Af(atEo>KWZjBUNma zs+C$%4j_3b&Z&a^;(4ogGz9O2l5iN%e)-{sM+Z$z3wuGWdjTGc zHQ^+reF}lZ>-*lzm%6Hw*CgdMj3NnDtQVbfv7vsJI#b?Fe9?&|Mq*DkD_iA9PO(;r z&5pzzXbzg4w^EH0()xrx3D~C(c7sVKEqfj^zAU$3!;bG-z_1NM2;1Zq$#o8VUof^{ zI1s@p81Y9S8}!4^K-g2a9B2ph&KrNi<6T}$Z7)8}K{uFa6T_xw^G4OGtyW9hl#6bX zW0Z3#M%!ZQHUk+A<`$T8^;{$QGMS`-L8@+&fqJvJAz5bTL2V;#O{JgRpW3Ng_qHZj z4`RNeSDB&C05M!b^DgX!ra5B?K7>-Z3@4!sXWDQjoM|teV{>MX5xN7?3H!oaqsc;( zkR?KF23hEr*Ilo`8qxYZAtf@$QuKy>lES(>Mg9!x8U~@+D48#}tmzFl_Su=1&MhniNz9c**8mYWHN4k z!kNg#YNThNXE51|M~~|VpFRkfI=W46;<06N4xD_DN14yX!m-@Z(a8}A{0&?-f8u$< zxH*bnhnd|s)1=%d%W-nAkWRfjNIKcwtu)HrtqRBl^IaWX9Uc8ZGS2BBG6h7qf|)=G zprQbvZ9uaf13d#ld~Xop2xfv=E+D46S59iUWoeX~WpQG-VL^(AFG$YQ5$GsqM@J7w zN052EATdWLuvWLM$&SLtOwNvz%Y;oB3n#A@wq$Y%o%~GLG8pIyH?ZDFN2hW}ut7kF zz$|of3IZFG3^ohQ1QS3lj*d~rUS%l}U_Mw2I6NF}Z71&%ky8Q(p=1s)?$8j!JU#}7 I0LLIh03FPXKL7v# diff --git a/tests/fixtures/golden_masters/paired_xbar_s/S_metadata.json b/tests/fixtures/golden_masters/paired_xbar_s/S_metadata.json index 3a20279..9281563 100644 --- a/tests/fixtures/golden_masters/paired_xbar_s/S_metadata.json +++ b/tests/fixtures/golden_masters/paired_xbar_s/S_metadata.json @@ -1,6 +1,7 @@ { "center_col": "center", "chart_type": "S", + "group_col": "group", "n_mode": "actual", "n_sigma": 3.0, "value_col": "s" diff --git a/tests/fixtures/golden_masters/paired_xbar_s/Xbar_data.parquet b/tests/fixtures/golden_masters/paired_xbar_s/Xbar_data.parquet index 3dfcb8ddf4c2365c8fbdfd4bdf925d53aee95011..39bd7e6b1d2d2387d192f2557d28a9f0e003c4da 100644 GIT binary patch delta 823 zcmdm`c1df4I%AB1sGg4~n<#@21A_z$GXp214HE+dus~^122mAJ4INPrQ30SrHW>*H zn2ZDmOsyD$1{TrD@0n#b1MOhu?J;GLU@K0}NX<;Dc8O>54JN(G&g@bkU6a+>_*tLvsZHaXY|WO; zdP_j9M_}?1W|%z7uz*CR#g|Bmd8l!Sy%1)Q)RAB-NX$z~ES9*& zvAL8zlTjp3Nk>5`Ei*MIB|a}PH&w|>K`C$YJ}&3U8#sA3-{Y9Y#C1kM9|Wo#-6uD3 z+cJ5UPd>=4%;%Kt#8Q}|LCxiH|j)9Jj`aqVWZ#s~01}Sn(0uint z0z#SvnCt7819^d-K-G?po*)9G5ei&DOn0xG)NsqvC^yUE#Bjrc6c3Ocj`<)z0BKG~ z$50U8F@5qeK@-N{$-f0nm^@r28wyzhqdHs2zP>5|s1nH%53q8CC1Cs9O+cbR`^z1j z9UXy6K#m3jC$LIjz_@|sBY|FZ1iJv}511iNPC;O`AfH3R5X=Q>ag=q8GWIGr@iz^pg>$2IJ_X*?X#r!GE z^-k$PaSx}Aq}-s4lP8D?GRDy<1ZaZ=km2g=3gSD1 zG&zEqAcmu(0T4T7Mn&Y50nJTIa?dGAj0ma%$v8VY`Z_wMJ35s+g3RLui8(rkf(WP3 z$^F76jA@gP2%9i@hfMw@Y{_Ua*+j%X0BAB$1yEDEqlcp-(3Oskfe@E~n67SMn}H^R rI3NN7K!!TyCFfciCW8gQTEOy?`9)P^a)2>}h8Q;SF)#!;1{neX^s|Jm diff --git a/tests/fixtures/golden_masters/paired_xbar_s/Xbar_metadata.json b/tests/fixtures/golden_masters/paired_xbar_s/Xbar_metadata.json index 1b12382..917d8c9 100644 --- a/tests/fixtures/golden_masters/paired_xbar_s/Xbar_metadata.json +++ b/tests/fixtures/golden_masters/paired_xbar_s/Xbar_metadata.json @@ -1,6 +1,7 @@ { "center_col": "center", "chart_type": "Xbar", + "group_col": "group", "n_mode": "actual", "n_sigma": 3.0, "value_col": "xbar" diff --git a/tests/fixtures/golden_masters/residual_chart/S_data.parquet b/tests/fixtures/golden_masters/residual_chart/S_data.parquet index 60e864407cd5e94bf439343cbe944ac65ed0cc8c..abf34eb1d4c044f72e656a4c7b217661f035c377 100644 GIT binary patch delta 780 zcmZWn&uFRTa5?90xqq9L{clJ35oI8 zgZhZ+Q4^D%4Oe4&@Ziy74?Wo)dNKM3cyMN+G||b=DeP*PujPjdP4!F>HxpVYS;c;Wo=dbyh z?#O|c2E1}=RbC13fr-IOeqQ)ySN81SV`K1>PYb&jlp_Z)mH=EAmf??JgSWz_FmNj; zF2d$7p8BPPVw2u`kQ@zKI+b><(hYvM4J(!nCeRK?r^1gm>do52cBNGhFHVKq@X@Xf zzgSzWkd7r{vygDa90%Sw9X%rl4aW^OZ@|9evRgG&j4_?~Ce98I9VZN6*o+hibk!NDK63FerqkQC)1DbC7*Zn^=5+Bz}gFPlX(K{ zSTzl`yRh$_W7Q%Yc~eXce7*%%GoavG=oinlSuINq%?3$_lR=@nMN#(T&<_itYJg@Te`|0g8KyuMre8P~#B0!^x6acghXtraZ zXCR304I&)DOfbs@#B}${Ne#CwjdHUrP7F6JNb&Fm$$2^g9p&uk=;7!HGLIJ|=I8|0 z>XtP*O30YW*>Q5ekO^bqQRPG2i2msVo_mfYKd-gL4j^!QBi)mLRo52ab|v= cf)O}uC3Aq02|^+e3QRK0<6~e5a11g80H-36U;qFB diff --git a/tests/fixtures/golden_masters/residual_chart/S_metadata.json b/tests/fixtures/golden_masters/residual_chart/S_metadata.json index d02a6ab..9de048b 100644 --- a/tests/fixtures/golden_masters/residual_chart/S_metadata.json +++ b/tests/fixtures/golden_masters/residual_chart/S_metadata.json @@ -1,6 +1,7 @@ { "center_col": "center", "chart_type": "S", + "group_col": "group", "n_mode": "actual", "n_sigma": 3.0, "question_answered": "Is within-subgroup variation stable?", diff --git a/tests/fixtures/golden_masters/stratified_balanced/S_data.parquet b/tests/fixtures/golden_masters/stratified_balanced/S_data.parquet index 60e864407cd5e94bf439343cbe944ac65ed0cc8c..abf34eb1d4c044f72e656a4c7b217661f035c377 100644 GIT binary patch delta 780 zcmZWn&uFRTa5?90xqq9L{clJ35oI8 zgZhZ+Q4^D%4Oe4&@Ziy74?Wo)dNKM3cyMN+G||b=DeP*PujPjdP4!F>HxpVYS;c;Wo=dbyh z?#O|c2E1}=RbC13fr-IOeqQ)ySN81SV`K1>PYb&jlp_Z)mH=EAmf??JgSWz_FmNj; zF2d$7p8BPPVw2u`kQ@zKI+b><(hYvM4J(!nCeRK?r^1gm>do52cBNGhFHVKq@X@Xf zzgSzWkd7r{vygDa90%Sw9X%rl4aW^OZ@|9evRgG&j4_?~Ce98I9VZN6*o+hibk!NDK63FerqkQC)1DbC7*Zn^=5+Bz}gFPlX(K{ zSTzl`yRh$_W7Q%Yc~eXce7*%%GoavG=oinlSuINq%?3$_lR=@nMN#(T&<_itYJg@Te`|0g8KyuMre8P~#B0!^x6acghXtraZ zXCR304I&)DOfbs@#B}${Ne#CwjdHUrP7F6JNb&Fm$$2^g9p&uk=;7!HGLIJ|=I8|0 z>XtP*O30YW*>Q5ekO^bqQRPG2i2msVo_mfYKd-gL4j^!QBi)mLRo52ab|v= cf)O}uC3Aq02|^+e3QRK0<6~e5a11g80H-36U;qFB diff --git a/tests/fixtures/golden_masters/stratified_balanced/S_metadata.json b/tests/fixtures/golden_masters/stratified_balanced/S_metadata.json index 3a20279..9281563 100644 --- a/tests/fixtures/golden_masters/stratified_balanced/S_metadata.json +++ b/tests/fixtures/golden_masters/stratified_balanced/S_metadata.json @@ -1,6 +1,7 @@ { "center_col": "center", "chart_type": "S", + "group_col": "group", "n_mode": "actual", "n_sigma": 3.0, "value_col": "s" diff --git a/tests/fixtures/golden_masters/stratified_balanced/Xbar_data.parquet b/tests/fixtures/golden_masters/stratified_balanced/Xbar_data.parquet index c89720b5084c1fe20efd6a3c0f3bc738b0934fc0..00eb8f6a982666a27036a5d157a0993c2a0ac960 100644 GIT binary patch delta 820 zcmX@Fa#3T#V}Air5g$<|Q3fFf1`Zi!1_%&k5ET*?(-Acg6%b_;Ws{NMfXN_OVhkEs zL?;U}3vYb9l}WwFltF^6I5{IVH&K+2L6k?8NrE-KD8IBomBB{_Xp1NdP=qmW@^)r7 z#+u3hn3G&tBuyFE1SEliqD*22VtW|HjxefyU{*W8Op5*0teTTESOi$RIMiBLC)cv1 zvA*I|+r>Hg1xqsPI&QUpT$2M>lUYCUs2$>&yn{8D^#Y$-8{cFtHc#KW)6PLAi&<2oat4+2$=?vtl;*)n;SPrk;b%;%Kt)5qB{Gbt~uB-tY;EyW|JJSy1J%+uMcASo}< z$Zzujo_J<)PO=7=G%*oFkN^WifMbv$0LD?lp8x;= delta 499 zcmcbpab9J@28xxaoaLEr%&eOQRZ`WbaI4X#{gfK%~m{57&kBCGht@;%`_?Z z$#R^0S5ULwDIKWB!zm*vHz*^y9H<<`cM1gRbO{8q9P>eh2Z-t931WhbbaV;<+GYV{ zxH`Ln_|70rj$kH;;pk`p#7>z}5jkZ*bJLRCb4n5;f~r6=&W?`0j*jV$PUVgu^LRmG zj*g)q!YOoepO6V-+T_DRCQRNTlYa_XG8#-a7Pb!nnhaC{)RgY%;phl-rK4jY#3dl6 zs~gy6pot(3h=2f)p-y?pxt4~>U;(feu>52`5jiDr@JZ$X;|mQjY~o{J2yhHC1OUDJ BhwlIY diff --git a/tests/fixtures/golden_masters/stratified_balanced/Xbar_metadata.json b/tests/fixtures/golden_masters/stratified_balanced/Xbar_metadata.json index 1b12382..917d8c9 100644 --- a/tests/fixtures/golden_masters/stratified_balanced/Xbar_metadata.json +++ b/tests/fixtures/golden_masters/stratified_balanced/Xbar_metadata.json @@ -1,6 +1,7 @@ { "center_col": "center", "chart_type": "Xbar", + "group_col": "group", "n_mode": "actual", "n_sigma": 3.0, "value_col": "xbar" diff --git a/tests/fixtures/golden_masters/stratified_uneven/S_data.parquet b/tests/fixtures/golden_masters/stratified_uneven/S_data.parquet index 34c13ed058c670558ae0d0a67443747179bef578..6d57ecf4d01f9944ac85c788fdfa016b4c98e9a1 100644 GIT binary patch delta 850 zcmb7BO-~a+7@pZLwxuoGrn7EMLpCKWe$|TJ;Ung(m%T4FarjX9Asia zZoTneArkq8$!|POy5D}>5Q4i|WGIbNb-7j~A)sPZk$MFK*I{0C@HXs;Zcd72k)p*| zlL)%0pJ4q5whlz=tMDIEAS_>w;D*PrFY50_>$_romLQK)5S1;cND=viYCTusiByuG z`mIksIFzGsO}NPBS^1>XdesRpg%oVdH{^q$wHsvfsS8J%Zd$5-6hg72p{y6{ilg;J(oC$ZF0YmE)r+;|#Hg95L&@jBp&DpD>nI9xHr1aRfSVF+p0`J zpZ~frRe=?MPspy=lv2LvO`2=&`CAyf5D3O_210=tTa* z%;xm{9PD=`cc@)qTPN+RTbrwvHW?LLG{+goGn{pcyIUM&H+Wj$&hz7q^uu(T6*jGU zO%6Jh;*~Vd!`<3q)*Z|~xHI@@-CbJi=VwsoF}ukyOMKM!2%X!oq74c547}DdxD3bI zh~QM9Gd!|0bB4`1IZo&aWD3uXr}hRf9ZF{?t#HWH`FO$03j828pP;lv$9aq0@SLQu x;Hj8wL%qNutQ)5jUKX#YGntcP?DG5_IXxdnfHmHy&obd*0tYXXfsi7c+PEN(=Z> z1}hl(5Hds&IXEOa#ugBdMj{-Y7-1bx1+jruEMs{}SEQqet{mrTXyzYYvcYSw_nz`+ zcOyubyOry;W(kiY?4IUzseZHF>rA7}6m5n@>fu`Cwvgnu$w#4_5d9G(tA6Ulyk=;> zIPC{#z8B2z{QvN`#d9Pis?u-SJQT?aH%k(dN$!b0X*O08s&2GO z&065H5onS7j`;B9*c0BluZEZmAVt=l^Ssq351psSW1BGmU@s&KiQy;bH%=%wEONtz zRjV0pWa7}Mq>{a2DmftQ6Miyu`GiinMV@mUXSIK8J%%>NL#~Y2LP;gYqguS;)`*0qHxln*aa+ delta 480 zcmZ3hF->K{_RV^XE12YKOc^BDijy-^a}z~*7({tgnIu@#i}FhgR2h6^7#Jr1Wp-oC zne4)nG?|B0g1v`T?FEyJ#AHWS;mLNafh_mf)YeTlVbY(>!Y(!W4y!clDGs$y?34en zCbRD0QhUNVIfX5kbsmq}HSWpR*gVA#@rx;_afof;Wsua7U@J(>OGzx2_{_f9j6IWa zGZW`5Cawm4eGo8BcbmM7+m^{WeezRoWj;qoCr1c&4DfZ?EXMPMadRDC3Uj?vI#Arh zDI+O2C?mNXs074!3IwWl2?Vkn^Ff3Mi0R}BVuFlubP565U;$*fI=h1S&LB;WU?zy+ z=x6}MPMJ{=Ib}d|(~{hCN)jW2sz5T%j*h;Lj_Hn0<&GfpctK*0j-epJDRgp{kO^bj zPBqhlb%B_O7&8`x%`i69P$fB=x8 kPI<|>mWIh-0k9UZ{Nz`{Dl$31$U#F4oA?+Q0vv-30jth~Jpcdz diff --git a/tests/fixtures/golden_masters/stratified_uneven/Xbar_metadata.json b/tests/fixtures/golden_masters/stratified_uneven/Xbar_metadata.json index 1b12382..917d8c9 100644 --- a/tests/fixtures/golden_masters/stratified_uneven/Xbar_metadata.json +++ b/tests/fixtures/golden_masters/stratified_uneven/Xbar_metadata.json @@ -1,6 +1,7 @@ { "center_col": "center", "chart_type": "Xbar", + "group_col": "group", "n_mode": "actual", "n_sigma": 3.0, "value_col": "xbar" diff --git a/tests/test_result_tabular.py b/tests/test_result_tabular.py index e747a63..6274451 100644 --- a/tests/test_result_tabular.py +++ b/tests/test_result_tabular.py @@ -7,6 +7,8 @@ from __future__ import annotations +from pathlib import Path + import pandas as pd import pytest @@ -65,8 +67,8 @@ def test_unknown_chart_raises(self, xbar_s_result): build_chart_table(xbar_s_result, chart="DoesNotExist") def test_columns_match_documented_contract(self, xbar_s_result): - """Output has value, center, lpl, upl, (signal). `subgroup` only when - chart_data has an `rsg` column (chart-type dependent).""" + """Output has value, center, lpl, upl, (signal). Xbar/S always add + `subgroup` and `n` (see TestXbarSSubgroupAndN).""" table = build_chart_table(xbar_s_result, chart="Xbar") assert "value" in table.columns assert "center" in table.columns @@ -131,6 +133,98 @@ def test_x_mr_chart_works(self, x_mr_result): assert len(table) > 0 +REFERENCE_CSV = Path(__file__).resolve().parent.parent / "validation" / "PBTESTDATABASE_T100.csv" +FACTORS = ["FACTOR 1", "FACTOR 2"] +TIME = "PRODUCTION TIME" + + +@pytest.fixture(scope="module") +def reference_studies(): + """Bishop's reference data: PM SDS 1 (balanced) and PM SDS 3 (cell sizes vary).""" + df = pd.read_csv(REFERENCE_CSV, na_values=["*"]) + return { + response: ProcessBehavior(df).formulate(response=response, factors=FACTORS, time=TIME) + for response in ("PM SDS 1", "PM SDS 3") + } + + +def _expected_n(study, by, column, min_n=1): + """Independent count of `column` per subgroup of `by`, keyed like the table's label.""" + counts = study.dataset.groupby(by, observed=True)[column].count() + counts = counts[counts >= min_n] + labels = ["_".join(map(str, k)) if isinstance(k, tuple) else str(k) for k in counts.index] + return dict(zip(labels, counts.tolist(), strict=True)) + + +class TestXbarSSubgroupAndN: + """Every Xbar/S table labels its subgroups and gives each one's own n (#143). + + `n` comes from the Xbar/S aggregation, so a subgroup that pools several cells + reports all of them — a per-cell size, which the table used to re-join, cannot. + """ + + POOLED = [ + ["FACTOR 1", TIME], + ["FACTOR 2", TIME], + ["FACTOR 1", "FACTOR 2", TIME], + ["FACTOR 1", "FACTOR 2"], + ["FACTOR 1"], + ] + + @pytest.mark.parametrize("response", ["PM SDS 1", "PM SDS 3"]) + @pytest.mark.parametrize("by", POOLED, ids=lambda b: "+".join(b)) + def test_by_tables_have_subgroup_and_true_n(self, reference_studies, response, by): + study = reference_studies[response] + result = study.execute(chart="Xbar", by=by, companion=True) + for chart, min_n in (("Xbar", 1), ("S", 2)): + table = build_chart_table(result, chart=chart) + assert list(table.columns[:2]) == ["subgroup", "n"] + assert dict(zip(table["subgroup"], table["n"], strict=True)) == _expected_n(study, by, response, min_n) + + @pytest.mark.parametrize("by", POOLED, ids=lambda b: "+".join(b)) + def test_solo_s_matches_companion_s(self, reference_studies, by): + study = reference_studies["PM SDS 3"] + companion = build_chart_table(study.execute(chart="Xbar", by=by, companion=True), chart="S") + solo = build_chart_table(study.execute(chart="S", by=by), chart="S") + pd.testing.assert_frame_equal(companion, solo) + + @pytest.mark.parametrize("response", ["PM SDS 1", "PM SDS 3"]) + def test_default_and_stratified_tables_give_cell_sizes(self, reference_studies, response): + study = reference_studies[response] + cell_n = _expected_n(study, ["rsg", TIME], response) + default = build_chart_table(study.execute(chart="Xbar"), chart="Xbar") + assert list(default.columns) == ["subgroup", "n", "value", "center", "lpl", "upl", "signal"] + assert dict(zip(default["subgroup"], default["n"], strict=True)) == cell_n + + stratified = study.execute(chart="Xbar", by=[TIME]) + table = build_chart_table(stratified, chart="Xbar") + data = stratified.get_chart("Xbar") + labels = data["rsg"].astype(str) + "_" + data[TIME].astype(str) + assert dict(zip(labels, table["n"], strict=True)) == cell_n + + def test_n_is_what_the_limits_rest_on(self, reference_studies): + """R5's limits rest on R2, undefined at the stream's first value: that cell counts one fewer.""" + study = reference_studies["PM SDS 3"] + table = build_chart_table(study.execute(chart="Xbar", value="R5"), chart="Xbar") + assert dict(zip(table["subgroup"], table["n"], strict=True)) == _expected_n(study, ["rsg", TIME], "R2") + first = table.iloc[0] + assert first["n"] == _expected_n(study, ["rsg", TIME], "PM SDS 3")[first["subgroup"]] - 1 + + def test_n_mode_average_keeps_actual_sizes(self, reference_studies): + study = reference_studies["PM SDS 3"] + by = ["FACTOR 1", TIME] + table = build_chart_table(study.execute(chart="Xbar", by=by, n_mode="average"), chart="Xbar") + assert dict(zip(table["subgroup"], table["n"], strict=True)) == _expected_n(study, by, "PM SDS 3") + + @pytest.mark.parametrize("by", [None, ["FACTOR 1", TIME], ["FACTOR 1"], [TIME]], ids=str) + def test_chart_data_carries_n_after_the_label(self, reference_studies, by): + result = reference_studies["PM SDS 3"].execute(chart="Xbar", by=by, companion=True) + for chart in ("Xbar", "S"): + columns = list(result.get_chart(chart).columns) + assert columns[columns.index("n") - 1] == (result.charts[chart]["metadata"].get("group_col") or TIME) + assert columns.index("n") < columns.index("center") + + # --------------------------------------------------------------------------- # build_signals_table # ---------------------------------------------------------------------------