fix(data-preparation): a missing factor value no longer makes a multi-factor study fail - #125
Merged
Merged
Conversation
…-factor study fail ## Summary - **What:** rows with no value in any factor column leave the analysis before the composite subgroup label is built, for one factor or many, with a ProcessBehaviorWarning naming the count per factor. - **Why:** with one factor such rows were dropped silently by the later rsg dropna; with two or more, _add_composite_column validated before that drop and raised "Cannot build RSG ... missing values in factor columns". The same file formulated with either factor alone and failed with both (Tom's survey file: 85,101 rows, 7,980 blank SURVEY QUESTION). - **Scope:** data_preparation.py (one helper, called before _add_grouping_column); tests/test_na_in_factor_columns.py; CHANGELOG. ## Contract / Invariants (must remain true) - Files with complete factor columns: byte-identical results, no warning (pinned). - Single-factor studies drop exactly the rows they dropped before; the composite check in _add_composite_column stays as a safety net. - ODS detection on raw data is unchanged (its own detector already filters NA factor rows; pinned by test_na_in_factor_is_filtered). - No chart math; validation/e2e_bishop_report.py exit 0. ## Behavior Changes (explicit) - formulate() with >=2 factors and a blank factor value now succeeds, dropping those rows, instead of raising. - One ProcessBehaviorWarning per formulate() when any factor value is missing: "Dropped 7,980 of 85,101 rows with no value in a factor column: SURVEY QUESTION (7,980 missing). ..." ## Tests - two factors with blanks formulate; one- and two-factor paths drop the same rows and create no 'nan' level; the warning carries per-column counts and names only columns with gaps; no warning when complete; blanks in both factors are named per column. ## Manual Verification - pytest tests/: 2365 passed, 10 skipped; ruff clean; mypy 17 pre-existing - validation/e2e_bishop_report.py: exit 0 - Tom's survey file, both factors: ADS 1, K 20, T 12, 77,121 rows, Xbar/S execute
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
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
ProcessBehaviorWarningnaming the count per factor.rsgdropna. With two or more,_add_composite_columnvalidated for missing values before that drop and raisedValidationError: Cannot build RSG ... missing values in factor columns. The same file formulated with either factor alone and failed with both. Found on a real survey file: 85,101 rows, 7,980 with a blankSURVEY QUESTION, formulated withfactors=["SURVEY QUESTION", "HOSPITAL"].data_preparation.pycalled before_add_grouping_column; a new test file; CHANGELOG.Contract / invariants
validation/e2e_bishop_report.pyexits 0.obs_idis stamped before the drop, so lineage to source rows holds._add_composite_columnstays as a safety net with its message intact.test_na_in_factor_is_filtered).Behaviour changes
formulate()with two or more factors and a blank factor value now succeeds, dropping those rows, instead of raising.ProcessBehaviorWarningperformulate()when any factor value is missing, e.g.Dropped 7,980 of 85,101 rows with no value in a factor column: SURVEY QUESTION (7,980 missing). A row with no factor value belongs to no cell, so it is excluded from the analysis.Only columns that actually have gaps are named.Methodology
Tests
tests/test_na_in_factor_columns.py:test_two_factors_formulate_with_missing_factor_values— formulates, drops exactly the blank rows, K = 20 with no phantom level.test_one_factor_and_two_factors_drop_the_same_rows.test_the_drop_is_reported_with_counts— the warning text, per-column counts, and that a complete column is not named.test_no_warning_when_factor_columns_are_complete.test_missing_values_in_both_factors_are_named_per_column.pytest tests/— 2365 passed, 10 skippedruff check .— cleanmypy processbehavior— 17 pre-existing errors, none newGolden masters untouched
Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_01BKphCi1LC1hjGYteWEK9JV