From 3ac08da3f33d23ff2f268386e965e5528de49619 Mon Sep 17 00:00:00 2001 From: Chris Nicholas <4948774+cnicholas@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:44:54 -0400 Subject: [PATCH 1/2] fix(signals): detect_signals() works on phased charts (#141) detect_signals() raised TypeError on every phased=True chart. Each phase has its own centre, so the statistics carry center=None, and the per-row zone calculation subtracted that scalar. The chart marks run rules not applicable (metadata run_rules_applicable=False, 70503be), but only the renderer read the flag. - result_signals passes run_rules_applicable to SignalDetector.detect. When it is False and the rules are the chart-type default, the default is Rule 1 only: rules 2-8 are not applicable rather than skipped, so the result is not marked partial on their account. - An explicit rule list runs as given (SignalConfig's "explicit rules, no filtering" contract). The per-row zones, the run rule (rule 4) and the violation records read each row's own centre when the scalar centre is None. The limits and the points the chart flags were always correct; Rule 1 now reports the same points (flagged_observations equals the rows with beyond_limits != 0). Unphased charts are unchanged: X still evaluates rules 1-8, Xbar/S Rule 1. tests/test_phased_limits.py TestPhasedSignals: combined, companion and stratified (via focus) phased charts; the violation records carry the phase's centre and limits; explicit rules 2 and 4 judged against each phase's centre; unphased X keeps every rule. --- CHANGELOG.md | 6 +++ processbehavior/analysis_result.py | 4 ++ processbehavior/result_signals.py | 2 + processbehavior/signals/detector.py | 33 ++++++++++++- processbehavior/signals/detectors.py | 3 ++ tests/test_phased_limits.py | 74 ++++++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a39165..a95766f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,12 @@ Equation numbers below are that manual's. one assignment, which builds the same figure (0.03 s for 800). Above 1,000 (`plotting.lane_boundaries.MAX_LANE_DIVIDERS`) none are drawn: the dividers would merge into a solid band at any chart width. +- **`detect_signals()` works on phased charts.** It raised `TypeError` on every `phased=True` + chart: each phase has its own centre, so the statistics carry `center=None`, and the zone + calculation read that scalar. The chart already marks run rules not applicable, but only the + plot read the flag. Phased charts now get Rule 1 by default, judged against each phase's + limits, with the same points the chart flags; an explicit rule list runs as given, against each + row's own centre. The limits and the flagged points on the chart were always correct. - **Xbar and S charts with subgroups of unequal size follow the manual (Eqs 11-5, 11-16, 11-17).** sigma-hat is the average of each subgroup's S_r / c4(N_r) (was S-bar / c4(N_r)), and each subgroup's limits use its own N_r, so they step with subgroup size, as VAS draws them. N_r now diff --git a/processbehavior/analysis_result.py b/processbehavior/analysis_result.py index 9bf8619..4a86b39 100644 --- a/processbehavior/analysis_result.py +++ b/processbehavior/analysis_result.py @@ -1039,6 +1039,10 @@ def detect_signals( ``signals.is_partial`` is True and ``signals.summary`` says which rules were evaluated. + On a phased chart (``phased=True``) each phase has its own centre line, so + the default rules are Rule 1 only, judged against each phase's limits. An + explicit rule list still runs as given, against each row's own centre. + Access violations: >>> signals.by_rule['rule_2'] # Rule 2 violations diff --git a/processbehavior/result_signals.py b/processbehavior/result_signals.py index b561ab1..37f247d 100644 --- a/processbehavior/result_signals.py +++ b/processbehavior/result_signals.py @@ -146,4 +146,6 @@ def _detect_for_chart( value_col=value_col, chart_name=chart_name, chart_type=chart_type, + # Phased charts mark run rules not applicable; until now only the renderer read it. + run_rules_applicable=metadata.get("run_rules_applicable", True), ) diff --git a/processbehavior/signals/detector.py b/processbehavior/signals/detector.py index 414f056..8bb2753 100644 --- a/processbehavior/signals/detector.py +++ b/processbehavior/signals/detector.py @@ -91,6 +91,7 @@ def detect( value_col: str = 'mean', chart_name: str = 'Chart', chart_type: str = 'Xbar', + run_rules_applicable: bool = True, ) -> SignalResult: """ Detect signals in control chart data. @@ -109,6 +110,11 @@ def detect( Name of chart for reporting chart_type : str, default 'Xbar' Chart type ('Xbar', 'S', 'X', 'mR') - determines applicable rules + run_rules_applicable : bool, default True + False for charts whose centre moves along the series (phased limits, + ``metadata['run_rules_applicable']``). The chart-type default rules + narrow to Rule 1; an explicit rule list runs as given, against each + row's own centre. Returns ------- @@ -126,6 +132,13 @@ def detect( # Get applicable rules for this chart type applicable_rules = config.get_rules_for_chart(chart_type) + # Phased limits give each phase its own centre, so the chart marks run rules + # 2-8 not applicable: they are not part of the default set, rather than + # skipped (a skip would mark the evaluation partial). An explicit rule list is + # the analyst's choice and runs as given. + if not run_rules_applicable and config.enabled_rules == 'default': + applicable_rules = [r for r in applicable_rules if r == 'rule_1'] + # Log rule selection if logger.isEnabledFor(logging.INFO): all_rules = [f'rule_{i}' for i in range(1, 9)] @@ -291,7 +304,7 @@ def _build_result( 'rule_number': int(rule_name.split('_')[1]), 'description': self.RULE_DESCRIPTIONS[rule_name], 'value': data.loc[idx, value_col], - 'center': stats['center'], + 'center': self._row_center(data, stats, idx), 'upl': ucl, 'lpl': lcl, } @@ -309,6 +322,22 @@ def _limits_vary(self, stats: dict) -> bool: """Check if control limits vary (per-row limits).""" return bool(stats.get('limits_vary')) + @staticmethod + def _center(data: pd.DataFrame, stats: dict): + """The centre line: the scalar, or the per-row column when there is none. + + Phased charts give each phase its own centre, so their statistics carry + ``center=None`` and the per-phase value lives on each row. + """ + if stats.get('center') is None and 'center' in data.columns: + return data['center'] + return stats['center'] + + def _row_center(self, data: pd.DataFrame, stats: dict, idx): + """The centre line at one row.""" + center = self._center(data, stats) + return center.loc[idx] if isinstance(center, pd.Series) else center + def _calculate_per_row_zones(self, data: pd.DataFrame, stats: dict, config: SignalConfig) -> pd.DataFrame: """ Calculate per-row zone boundaries for varying process limits. @@ -334,7 +363,7 @@ def _calculate_per_row_zones(self, data: pd.DataFrame, stats: dict, config: Sign if 'upl' not in data.columns or 'lpl' not in data.columns: raise ValueError("Cannot calculate per-row zones: data missing 'upl' or 'lpl' columns") - center = stats['center'] + center = self._center(data, stats) zone_def = config.zone_definition # Calculate sigma per row diff --git a/processbehavior/signals/detectors.py b/processbehavior/signals/detectors.py index 38e18e0..2c2431b 100644 --- a/processbehavior/signals/detectors.py +++ b/processbehavior/signals/detectors.py @@ -173,6 +173,9 @@ def detect_run(data: pd.DataFrame, stats: dict, value_col: str, length: int = 8) """ values = data[value_col] center = stats.get('center', stats.get('Mean')) # Try 'center' first, fallback to 'Mean' + if center is None and 'center' in data.columns: + # Phased charts: each phase has its own centre, carried on the rows. + center = data['center'] # Above or below center above = (values > center).astype(int) diff --git a/tests/test_phased_limits.py b/tests/test_phased_limits.py index ff451cf..d4c5cf6 100644 --- a/tests/test_phased_limits.py +++ b/tests/test_phased_limits.py @@ -412,3 +412,77 @@ def test_phased_residual_limits_vary(self, sds2_study): result = sds2_study.execute(chart='X', by=[], phased=True, value='R2') data = result.get_chart('X') assert data['center'].nunique() > 1 or data['upl'].nunique() > 1 + + +# ============================================================================= +# Phased + signal detection +# ============================================================================= + + +class TestPhasedSignals: + """detect_signals() on phased charts: Rule 1 against each phase's own limits. + + Phased statistics carry ``center=None`` (each phase has its own centre) and the + chart marks run rules not applicable. detect_signals() used to read the scalar + centre and raise TypeError on every phased chart. + """ + + def test_phased_x_rule_1_against_phase_limits(self, sds1_study): + result = sds1_study.execute(chart='X', by=[], phased=True) + signals = result.detect_signals(chart='X') + assert signals.rules_evaluated == ['rule_1'] + assert signals.rules_skipped == {} + assert not signals.is_partial + data = result.get_chart('X') + beyond = set(data.index[data['beyond_limits'] != 0]) + assert beyond, 'fixture should have a point beyond its phase limits' + assert signals.flagged_observations == beyond + + def test_violation_records_the_phase_centre(self, sds1_study): + result = sds1_study.execute(chart='X', by=[], phased=True) + data = result.get_chart('X') + violations = result.detect_signals(chart='X').to_dataframe() + assert violations['center'].notna().all() + for _, row in violations.iterrows(): + assert row['center'] == data.loc[row['obs_id'], 'center'] + assert row['upl'] == data.loc[row['obs_id'], 'upl'] + + def test_phased_companion_both_charts(self, sds1_study): + result = sds1_study.execute(chart='X', by=[], phased=True, companion=True) + signals = result.detect_signals() + assert set(signals) == {'X', 'mR'} + for name, sig in signals.items(): + data = result.get_chart(name) + assert sig.rules_evaluated == ['rule_1'] + assert sig.flagged_observations == set(data.index[data['beyond_limits'] != 0]) + + def test_phased_stratified_focus(self, sds1_study): + result = sds1_study.execute(chart='X', by=['factor 1'], phased=True, companion=True) + for stratum in result.strata: + signals = result.focus(stratum).detect_signals() + assert {sig.rules_evaluated[0] for sig in signals.values()} == {'rule_1'} + + def test_explicit_rules_run_against_each_phase_centre(self): + """An explicit rule list runs as given; run and zone rules read the row's centre.""" + from processbehavior.signals import SignalConfig, SignalDetector + + # Phase 1 sits above its centre 0, phase 2 below its centre 10: one run each. + # Phase 2's last two points fall in its lower zone A (7 to 8, sigma 1). + data = pd.DataFrame({ + 'x': [1.0] * 8 + [9.0] * 6 + [7.5, 7.5], + 'center': [0.0] * 8 + [10.0] * 8, + 'upl': [3.0] * 8 + [13.0] * 8, + 'lpl': [-3.0] * 8 + [7.0] * 8, + }) + stats = {'N': None, 'center': None, 'lpl': None, 'upl': None, 'limits_vary': True} + signals = SignalDetector().detect( + data, stats, SignalConfig(enabled_rules=['rule_2', 'rule_4'], min_observations=1), + value_col='x', chart_type='X', run_rules_applicable=False, + ) + assert signals.rules_evaluated == ['rule_2', 'rule_4'] + assert set(signals.by_rule['rule_4']['obs_id']) == {7, 15} + assert set(signals.by_rule['rule_2']['obs_id']) == {15} + + def test_unphased_x_keeps_every_rule(self, sds1_study): + signals = sds1_study.execute(chart='X', by=[]).detect_signals(chart='X') + assert signals.rules_evaluated == [f'rule_{i}' for i in range(1, 9)] From 5947ec5681a7aa9b9d76a879e31f6c20ee12a789 Mon Sep 17 00:00:00 2001 From: Chris Nicholas <4948774+cnicholas@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:46:08 -0400 Subject: [PATCH 2/2] fix(study): value='response' charts the response (#142) execute(value='response') is in the signature (Literal['response', 'R1', ...]) and the docstring, but every chart refused it: "Valid charts for RESPONSE: None". _validate_execute_request tells residuals apart by value.upper().startswith('R'), which 'RESPONSE' also satisfies, so the value went down the residual path and failed the pairing check. why_not() and supports_calibration() refused it the same way. _normalise_value turns 'response' (any case) into None, the response's own spelling, at the start of execute(), supports_calibration() and why_not(); _resolve_value_column already treats None as the response. recentered=True with the response is still refused, and the message now says why: the response is already on the measurement scale. tests/test_execute_validation.py TestValueResponse: 'response', 'Response' and 'RESPONSE' give the same charts, statistics and metadata as value=None on Xbar, S, Xbar+S, X, mR and Histogram; why_not and supports_calibration agree; recentered stays refused. --- CHANGELOG.md | 4 +++ processbehavior/study.py | 21 ++++++++++++- tests/test_execute_validation.py | 51 ++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a95766f..71cf7ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -125,6 +125,10 @@ Equation numbers below are that manual's. plot read the flag. Phased charts now get Rule 1 by default, judged against each phase's limits, with the same points the chart flags; an explicit rule list runs as given, against each row's own centre. The limits and the flagged points on the chart were always correct. +- **`value='response'` charts the response**, as the `execute()` signature and docstring say. + Every chart refused it ("Valid charts for RESPONSE: None"): residuals are told apart by a leading + `R`, which `RESPONSE` also has. `execute()`, `supports_calibration()` and `why_not()` now treat + it, in any case, exactly as `value=None`. - **Xbar and S charts with subgroups of unequal size follow the manual (Eqs 11-5, 11-16, 11-17).** sigma-hat is the average of each subgroup's S_r / c4(N_r) (was S-bar / c4(N_r)), and each subgroup's limits use its own N_r, so they step with subgroup size, as VAS draws them. N_r now diff --git a/processbehavior/study.py b/processbehavior/study.py index fda6421..2f4a935 100644 --- a/processbehavior/study.py +++ b/processbehavior/study.py @@ -75,6 +75,17 @@ def _base_residual_code(value: str) -> str: return upper[2:] if upper.startswith('RCR') else upper +def _normalise_value(value: str | None) -> str | None: + """'response' (any case) -> None, the response's own spelling; residuals unchanged. + + Residuals are told apart by their leading 'R', which 'RESPONSE' also has, so the + documented ``value='response'`` must become None before anything tests the prefix. + """ + if isinstance(value, str) and value.lower() == 'response': + return None + return value + + @dataclass class DesignReport: """ @@ -1521,6 +1532,8 @@ def why_not(self, chart: str, value: str | None = None) -> str: >>> study.why_not('Xbar', value='R2') "'Xbar' with value='R2' is not valid for ADS 1.\\nValid charts for R2: S, X" """ + value = _normalise_value(value) + # Handle old fused syntax: why_not('R5_Xbar') -> why_not('Xbar', value='R5') if '_' in chart and value is None: parts = chart.split('_', 1) @@ -2018,6 +2031,8 @@ def execute( ) by = stratify_by + value = _normalise_value(value) + # 0. Resolve calibration (label string -> attached Calibration object). calibration_obj = self._resolve_calibration(calibration) @@ -2114,6 +2129,7 @@ def supports_calibration( 'parameter.' ) by = stratify_by + value = _normalise_value(value) from .analysis import Analysis from .formulation_spec import ChartRequest @@ -2396,7 +2412,10 @@ def _validate_recentered(self, value: str | None) -> None: ) recenterable = set(RESIDUAL_CODES) if value is None: - raise ValidationError('recentered=True requires a residual value (R1-R6), got value=None') + raise ValidationError( + 'recentered=True requires a residual value (R1-R6); the response ' + '(value=None or "response") is already on the measurement scale.' + ) if value.upper() not in recenterable: raise ValidationError(f"recentered=True requires a residual value (R1-R6), got '{value}'") diff --git a/tests/test_execute_validation.py b/tests/test_execute_validation.py index 054d113..a54591c 100644 --- a/tests/test_execute_validation.py +++ b/tests/test_execute_validation.py @@ -218,3 +218,54 @@ def test_execute_with_accessor(self, sds1_study): """End-to-end: study.execute(chart=..., value=study.residuals.R5).""" result = sds1_study.execute(chart=sds1_study.charts.Xbar, value=sds1_study.residuals.R5) assert result is not None + + +# ============================================================================= +# value='response' is the response (#142) +# ============================================================================= + + +class TestValueResponse: + """The documented value='response' charts the response, like value=None. + + Residuals are told apart by a leading 'R', which 'RESPONSE' also has, so the + documented spelling was routed down the residual path and refused. + """ + + @pytest.mark.parametrize( + 'kwargs', + [ + {'chart': 'Xbar'}, + {'chart': 'S'}, + {'chart': 'Xbar', 'companion': True}, + {'chart': 'X', 'by': []}, + {'chart': 'mR', 'by': []}, + {'chart': 'Histogram'}, + ], + ids=['Xbar', 'S', 'Xbar+S', 'X', 'mR', 'Histogram'], + ) + @pytest.mark.parametrize('spelling', ['response', 'Response', 'RESPONSE']) + def test_same_result_as_none(self, sds1_study, kwargs, spelling): + from pandas.testing import assert_frame_equal + + expected = sds1_study.execute(**kwargs) + actual = sds1_study.execute(value=spelling, **kwargs) + assert set(actual.charts) == set(expected.charts) + for name in expected.charts: + assert_frame_equal(actual.get_chart(name), expected.get_chart(name)) + assert actual.get_statistics(name) == expected.get_statistics(name) + assert actual.charts[name]['metadata'] == expected.charts[name]['metadata'] + + def test_why_not_agrees(self, sds1_study): + for chart in ('Xbar', 'S', 'X', 'mR'): + assert sds1_study.why_not(chart, value='response') == sds1_study.why_not(chart) + + def test_supports_calibration_agrees(self, sds1_study): + for kwargs in ({'chart': 'X', 'by': []}, {'chart': 'Xbar'}, {'chart': 'X', 'by': ['factor 1']}): + assert sds1_study.supports_calibration(value='response', **kwargs) == ( + sds1_study.supports_calibration(**kwargs) + ) + + def test_recentered_response_still_refused(self, sds1_study): + with pytest.raises(ValidationError, match='recentered.*requires.*residual'): + sds1_study.execute(chart='Xbar', value='response', recentered=True)