diff --git a/CHANGELOG.md b/CHANGELOG.md index 560e99f..95c44a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 No arithmetic changed; none of these names is exported from the package top level. ### Fixed +- **A missing value in a factor column no longer makes a two-factor study fail.** With one + factor, rows with a blank factor value were dropped silently; with two or more, the + composite subgroup label was built before that drop and ``formulate`` raised + ``ValidationError: Cannot build RSG ... missing values in factor columns``. The same + file formulated with either factor alone and failed with both (a 85,101-row survey + file with 7,980 blank ``SURVEY QUESTION`` rows). Rows with no value in any factor + column now leave the analysis before the label is built, for any number of factors, + and the library says so with a ``ProcessBehaviorWarning`` naming the count per + factor. Response and time missing values are handled as before. - **``evaluate`` no longer raises on a constant column.** Binning a column with no spread (every value identical) returned the documented no-spread result for ``equal_freq`` but raised ``ValueError: Bin edges must be unique`` from ``pd.cut`` for ``equal_width`` and diff --git a/processbehavior/data_preparation.py b/processbehavior/data_preparation.py index 29c0718..0e3db11 100644 --- a/processbehavior/data_preparation.py +++ b/processbehavior/data_preparation.py @@ -16,6 +16,7 @@ from __future__ import annotations import logging +import warnings from functools import reduce from typing import TYPE_CHECKING @@ -24,7 +25,7 @@ from natsort import natsorted from pandas.api.types import is_numeric_dtype -from .exceptions import ColumnNotFoundError, FactorNotFoundError, ValidationError +from .exceptions import ColumnNotFoundError, FactorNotFoundError, ProcessBehaviorWarning, ValidationError if TYPE_CHECKING: from .formulation_spec import FormulationSpec @@ -281,6 +282,13 @@ def prepare_dataset(self, df: pd.DataFrame, spec: FormulationSpec) -> pd.DataFra # Add composite grouping variable if needed if spec.has_grouping: + # A row with no value in a factor column belongs to no cell, so it leaves the + # analysis here — for one factor or many. The single-factor path used to drop + # such rows silently via the rsg dropna below; the multi-factor path raised + # from _add_composite_column before reaching it, so the same file formulated + # with one factor and failed with two (Tom's survey file, 7,980 blank + # SURVEY QUESTION rows). Both paths now drop first and say so. + out = self._drop_rows_missing_factor_values(out, spec) out = self._add_grouping_column(out, spec) # Drop rows with missing values in analysis-critical columns BEFORE @@ -533,6 +541,30 @@ def build_keys(self, df: pd.DataFrame, spec: FormulationSpec) -> pd.DataFrame: # Private Helper Methods # ======================================================================== + def _drop_rows_missing_factor_values(self, df: pd.DataFrame, spec: FormulationSpec) -> pd.DataFrame: + """ + Drop rows with a missing value in any factor column, and report it. + + Reported as a ``ProcessBehaviorWarning`` (the house style for "the + library changed your data on your behalf"), naming the count per + factor, so an analyst sees that 7,980 rows left the study rather than + discovering a smaller N later. + """ + factor_cols = spec.rsg_vars_list + missing = df[factor_cols].isna() + if not missing.any().any(): + return df + per_col = {col: int(n) for col, n in missing.sum().items() if n} + n_rows = int(missing.any(axis=1).sum()) + warnings.warn( + f'Dropped {n_rows:,} of {len(df):,} rows with no value in a factor column: ' + + ', '.join(f'{col} ({n:,} missing)' for col, n in per_col.items()) + + '. A row with no factor value belongs to no cell, so it is excluded from the analysis.', + ProcessBehaviorWarning, + stacklevel=6, + ) + return df.loc[~missing.any(axis=1)] + def _add_grouping_column(self, df: pd.DataFrame, spec: FormulationSpec) -> pd.DataFrame: """ Add composite grouping column (e.g., 'lane_head'). diff --git a/tests/test_na_in_factor_columns.py b/tests/test_na_in_factor_columns.py new file mode 100644 index 0000000..c85b02f --- /dev/null +++ b/tests/test_na_in_factor_columns.py @@ -0,0 +1,96 @@ +"""Rows with a missing factor value leave the study, for one factor or many, with a warning. + +Found on a survey file: 85,101 rows, 7,980 with a blank SURVEY QUESTION. It formulated with +either factor alone and raised with both, because the two-factor path built the composite +subgroup label before the missing-value drop. Nothing in the app surfaced that for long +enough to read. +""" + +import warnings + +import numpy as np +import pandas as pd +import pytest + +import processbehavior as pb +from processbehavior import ProcessBehaviorWarning + + +@pytest.fixture +def survey(): + """Two hospitals x 10 questions x 12 months, 6 replicates, with 8% of question labels blank. + + Six replicates so that no cell falls below two after the blanks leave: the study stays + at full replication and the test checks the drop, not a change of design state. + """ + rng = np.random.default_rng(11) + rows = [] + for month in range(1, 13): + for hospital in ('A', 'B'): + for q in ('LR', 'NC', 'NI', 'NP', 'PC', 'PI', 'PT', 'QF', 'SA', 'SD'): + for _ in range(6): + rows.append( + {'MONTH': month, 'HOSPITAL': hospital, 'QUESTION': q, 'SCORE': rng.choice([0, 25, 50, 75, 100])} + ) + df = pd.DataFrame(rows) + blank = rng.random(len(df)) < 0.08 + df.loc[blank, 'QUESTION'] = np.nan + return df, int(blank.sum()) + + +def _formulate(df, factors): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + st = pb.formulate(df, response='SCORE', factors=factors, time='MONTH') + return st, [w for w in caught if issubclass(w.category, ProcessBehaviorWarning)] + + +def test_two_factors_formulate_with_missing_factor_values(survey): + df, n_blank = survey + st, pbw = _formulate(df, ['QUESTION', 'HOSPITAL']) + assert st.analytical_design_state.sds == 1 + assert len(st._ads.analysis_dataset) == len(df) - n_blank + assert st.design().K == 20 # 10 questions x 2 hospitals, no phantom 'nan' level + + +def test_one_factor_and_two_factors_drop_the_same_rows(survey): + df, n_blank = survey + one, _ = _formulate(df, ['QUESTION']) + two, _ = _formulate(df, ['QUESTION', 'HOSPITAL']) + assert len(one._ads.analysis_dataset) == len(two._ads.analysis_dataset) == len(df) - n_blank + assert 'nan' not in set(one._ads.analysis_dataset['rsg'].astype(str)) + + +def test_the_drop_is_reported_with_counts(survey): + df, n_blank = survey + _, pbw = _formulate(df, ['QUESTION', 'HOSPITAL']) + texts = [str(w.message) for w in pbw] + hit = [t for t in texts if 'no value in a factor column' in t] + assert hit, texts + assert f'Dropped {n_blank:,} of {len(df):,} rows' in hit[0] + assert f'QUESTION ({n_blank:,} missing)' in hit[0] + assert 'HOSPITAL' not in hit[0] # only the columns that actually had gaps are named + + +def test_no_warning_when_factor_columns_are_complete(survey): + df, _ = survey + complete = df.dropna(subset=['QUESTION']) + _, pbw = _formulate(complete, ['QUESTION', 'HOSPITAL']) + assert not [w for w in pbw if 'factor column' in str(w.message)] + + +def test_missing_values_in_both_factors_are_named_per_column(): + df = pd.DataFrame( + { + 'lane': ['A', 'A', None, 'B', 'B', 'B'], + 'head': [1, 1, 1, None, 2, 2], + 't': [1, 1, 1, 1, 1, 1], + 'y': [1.0, 2.0, 3.0, 4.0, 5.0, 6.0], + } + ) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter('always') + st = pb.formulate(df, response='y', factors=['lane', 'head'], time='t') + msg = next(str(w.message) for w in caught if 'factor column' in str(w.message)) + assert 'Dropped 2 of 6 rows' in msg and 'lane (1 missing)' in msg and 'head (1 missing)' in msg + assert len(st._ads.analysis_dataset) == 4