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 b35badc..a7b7237 100644 Binary files a/tests/fixtures/golden_masters/paired_xbar_s/S_data.parquet and b/tests/fixtures/golden_masters/paired_xbar_s/S_data.parquet differ 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 3dfcb8d..39bd7e6 100644 Binary files a/tests/fixtures/golden_masters/paired_xbar_s/Xbar_data.parquet and b/tests/fixtures/golden_masters/paired_xbar_s/Xbar_data.parquet differ 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 60e8644..abf34eb 100644 Binary files a/tests/fixtures/golden_masters/residual_chart/S_data.parquet and b/tests/fixtures/golden_masters/residual_chart/S_data.parquet differ 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 60e8644..abf34eb 100644 Binary files a/tests/fixtures/golden_masters/stratified_balanced/S_data.parquet and b/tests/fixtures/golden_masters/stratified_balanced/S_data.parquet differ 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 c89720b..00eb8f6 100644 Binary files a/tests/fixtures/golden_masters/stratified_balanced/Xbar_data.parquet and b/tests/fixtures/golden_masters/stratified_balanced/Xbar_data.parquet differ 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 34c13ed..6d57ecf 100644 Binary files a/tests/fixtures/golden_masters/stratified_uneven/S_data.parquet and b/tests/fixtures/golden_masters/stratified_uneven/S_data.parquet differ diff --git a/tests/fixtures/golden_masters/stratified_uneven/S_metadata.json b/tests/fixtures/golden_masters/stratified_uneven/S_metadata.json index 3a20279..9281563 100644 --- a/tests/fixtures/golden_masters/stratified_uneven/S_metadata.json +++ b/tests/fixtures/golden_masters/stratified_uneven/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_uneven/Xbar_data.parquet b/tests/fixtures/golden_masters/stratified_uneven/Xbar_data.parquet index 32e88cf..ee8048e 100644 Binary files a/tests/fixtures/golden_masters/stratified_uneven/Xbar_data.parquet and b/tests/fixtures/golden_masters/stratified_uneven/Xbar_data.parquet differ 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 # ---------------------------------------------------------------------------