Repository navigation
fix: detect_signals() on phased charts; value='response' (#141, #142) - #145
Merged
Merged
Conversation
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.
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.
This was referenced Oct 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs found while scoping phased Xbar/S (#144), both fixed for 0.4.0. Stacked on #138.
detect_signals()works on phased charts.TypeErroron everyphased=Truechart. Phased statistics carrycenter=None(each phase has its own centre), and the per-row zone calculation subtracted that scalar. The chart marks run rules not applicable, but only the renderer read the flag.value='response'charts the response.execute()signature and docstring, but every chart refused it ("Valid charts for RESPONSE: None"), because residuals are told apart by a leadingR.execute(),supports_calibration()andwhy_not()treat it, in any case, asvalue=None.Contract / invariants
value=Nonebehaves exactly as before.recentered=Truewith the response is still refused, now with a message that says why.Tests
tests/test_phased_limits.py::TestPhasedSignals:focus) phased charts;tests/test_execute_validation.py::TestValueResponse:'response','Response'and'RESPONSE'equalvalue=Noneon Xbar, S, Xbar+S, X, mR and Histogram (charts, statistics, metadata);why_notandsupports_calibrationagree;Verification
ruff check .: clean.docs/reference/validation.mdis unchanged when regenerated.Closes #141
Closes #142