diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a39165..71cf7ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,16 @@ 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. +- **`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/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/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) 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)]