diff --git a/CHANGELOG.md b/CHANGELOG.md index f6c8c19a..1263af8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,55 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `apply_correction_entries`) renders the action, new-value and reason inputs for a prefilled KEY/column/current value, with namespaced widget keys. The Correct Data page now uses it — #296 +- **Outliers and constraints corrections**: Each row of the constraint + violations and outlier inspection tables has a Review button (a pinned + `st.column_config.ButtonColumn`) that opens the shared correction form in a + dialog, prefilled with the row's KEY, column and current value, to modify the + value, remove it or accept it as valid (source and check type + `outliers`/`constraints`). Accepted flags are hidden and left out of the + metrics unless "Show reviewed" is on (then they are highlighted green), and + come back if the value changes or + the acceptance is removed. Accepting a hard violation needs a confirmation. + A "Show only flagged values" toggle (on by default) sits above both tables; + the outlier inspection table previously always listed unflagged values too. + With "Show reviewed" on, values whose current value comes from a modify or + remove correction are also highlighted green, with a "Corrected" badge and + the correction reason (new `CorrectionProcessor.get_active_corrections`); + unlike accepted flags, corrected values that are still flagged stay visible + and counted. A "Show only reviewed" toggle lists only accepted and + corrected rows and disables the other two toggles while on (toggle values + are a `review.TableFilters`, applied by `review.filter_table`). The outlier + inspection table now hides its index. Styled tables + are built with new `ui_utils.row_styler`, which keeps values displayed as in + the unstyled table (pandas' default Styler formatting showed `150` as + `150.000000` and missing values as `nan`). + Flag review logic lives in the new Streamlit-free + `src/datasure/checks/outliers/review.py`; `outliers_report` takes the + dataset `alias`. Removed the unused `_render_outlier_table`. + `queue_notice` gains a `toast` level, and `show_queued_notices` returns + whether it showed anything. New `ui_utils.ensure_styler_limit` raises + pandas' process-wide `styler.render.max_elements` under a lock and never + lowers it, so concurrent sessions can't cut it below what another render + needs; it replaces the `pd.set_option` calls in the summary, missing and + progress checks, which lowered the limit to fit their own tables and could + crash other styled tables. New `ui_utils.styled_dataframe` renders a Styler + after raising the limit to fit it; the results tables and the Correction + Log use it. A soft-violation acceptance no longer hides a value that has + since become a hard violation (e.g. after bounds are tightened): it needs a + new hard acceptance. A correction that failed to reapply to new prep data + is never shown as Corrected, even if the data holds its new value. Review + on a KEY whose rows hold different values of the column shows a warning + instead of the form, since a correction changes every row with the KEY + (`review.key_has_conflicting_values`). Survey fields added through "Show + more columns" that share a name with a results or review column get a + " (survey)" suffix, even while the review columns are hidden. A Survey KEY + named "review status" or "review reason" turns review off with a warning + instead of being overwritten — #298 +- **Correction log severity**: New `severity` column, `hard` on acceptances of + hard constraint violations (null otherwise and for legacy logs). + `CorrectionEntry.severity` sets it and is rejected on non-accept actions, + on acceptances of other checks, and with any value other than `hard`. + Hard acceptances are highlighted in the Correction Log — #298 ### Fixed @@ -43,6 +92,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 string still does; use "remove value" to blank a cell — #296 - **Correction log schema**: Removing the last correction entry now leaves an empty log with the full schema, including status columns — #296 +- **Constraint violations**: A value past a hard bound was reported as a soft + violation whenever a soft bound on the same side was set (for example, + above the hard maximum read "above soft maximum"), so hard violations were + undercounted. Hard bounds are now tested first + (`compute_constraint_violations`) — #298 ## [1.1.0] - 2026-09-21 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 13d31987..a9135bde 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -150,9 +150,13 @@ Every Streamlit view must render its chrome through the shared helpers in (delete/remove/restart) — do not invent per-view confirm flows with session-state flags, expanders, or inline warnings. - `queue_notice(scope, level, message)` for any success/warning/error message - raised just before an `st.rerun()` (including a `confirm_dialog` callback, - which reruns), with `show_queued_notices(scope)` where it should appear on - the next run. Rendering it directly gets cleared by the rerun. + or toast raised just before an `st.rerun()` (including a `confirm_dialog` + callback, which reruns), with `show_queued_notices(scope)` where it should + appear on the next run. Rendering it directly gets cleared by the rerun. +- Render a pandas Styler with `styled_dataframe`, or call + `ensure_styler_limit(cells)` before rendering it. Never set + `styler.render.max_elements` with `pd.set_option`: the option is shared by + every session, and lowering it can break another session's styled table. - Use `st.divider()` for horizontal rules, never `st.write("---")`. - Icons are Material shortcodes (`:material/check_circle:`), not emoji shortcodes (`:white_check_mark:`). diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index cbb2c6e0..0ffff2fe 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -469,6 +469,8 @@ Check type column shows which check it applies to. It stays in effect only while the value is unchanged. Accept entries are kept in `correction_log.csv` in the replication package but are not part of the corrections script. You can remove an accept entry with "Remove correction step" like any other entry. +Accepting a hard constraint violation sets Severity to `hard`, and those rows +are highlighted in red. To blank a cell, use "remove value": "modify value" needs a non-empty new value (`0` is valid). @@ -827,6 +829,45 @@ Visual analysis: - **Box Plot**: Distribution with outliers highlighted - **Table**: All records with outlier indicators +##### Correcting or Accepting Flagged Values + +Above each of the constraint violations and outlier inspection tables, +**Show only flagged values** (on by default) limits the table to flagged +values. Turn it off to see every checked value. + +Each row of the constraint violations table and the outlier inspection table +starts with a **Review** button. Click it to open a correction form in a +dialog, with the KEY, column and current value filled in. Choose an action, +enter a reason and click "Apply": + +- **modify value** or **remove value** corrects the data. The page reloads, and + the flag is updated or disappears. +- **accept** records that the flagged value is correct. The flag is hidden + and no longer counted in the metrics. Turn on "Show reviewed" to see + accepted flags highlighted in green, with a Reviewed badge and the reason. + +"Show reviewed" also highlights corrected values in green, with a Corrected +badge and the correction's reason. A corrected value that is now in range is +no longer flagged, so turn off "Show only flagged values" to see it. A +corrected value that is still flagged stays in the table and in the metrics +until it is fixed or accepted. The badge clears if the value changes again. + +Turn on **Show only reviewed** to list only accepted and corrected values. While +it is on, the other two toggles are disabled. + +Outlier and constraint acceptances are separate: accepting an outlier does not +accept a constraint violation on the same value. Accepting a **hard** +constraint violation needs an extra confirmation. An accepted flag comes back +if the value changes, or if you remove the acceptance on the Correct Data page. +Every entry appears in the Correction Log with source `outliers` or +`constraints`. + +Corrections and acceptances apply to every row with the same KEY. If a KEY is +on more than one row with different values in the flagged column, Review shows +a warning instead of the form, because a correction would change all those +rows. Give each record a unique KEY in the source data first. The Duplicates +check lists duplicated KEYs. + --- ### 6. Enumerator Stats Report diff --git a/src/datasure/checks/missing.py b/src/datasure/checks/missing.py index 07838e22..a573004b 100644 --- a/src/datasure/checks/missing.py +++ b/src/datasure/checks/missing.py @@ -29,6 +29,7 @@ save_check_settings, trigger_save, ) +from datasure.utils.ui_utils import ensure_styler_limit TAB_NAME = "missing" @@ -930,7 +931,7 @@ def missing_columns( if not mv_data_filtered.empty: cmap = sns.light_palette("pink", as_cmap=True) styler_limit = mv_data_filtered.shape[0] * mv_data_filtered.shape[1] - pd.set_option("styler.render.max_elements", styler_limit) + ensure_styler_limit(styler_limit) st.dataframe( mv_data_filtered.style.format( @@ -1114,7 +1115,7 @@ def missing_compare( else: cmap = sns.light_palette("pink", as_cmap=True) styler_limit = group_by_data.shape[0] * group_by_data.shape[1] - pd.set_option("styler.render.max_elements", styler_limit) + ensure_styler_limit(styler_limit) st.dataframe( group_by_data.style.format(subset=compare_col, precision=2) diff --git a/src/datasure/checks/outliers/compute.py b/src/datasure/checks/outliers/compute.py index d80339ba..90ac12ee 100644 --- a/src/datasure/checks/outliers/compute.py +++ b/src/datasure/checks/outliers/compute.py @@ -920,15 +920,17 @@ def compute_constraint_violations( for col in outlier_cols: col_df = data.select([survey_key, col]) + # Hard bounds are tested before soft ones: a value past a hard + # bound is also past the soft bound inside it. violation_expr = ( pl.when((hard_min is not None) & (pl.col(col) < hard_min)) .then(pl.lit(f"Value is below hard minimum {hard_min}")) + .when((hard_max is not None) & (pl.col(col) > hard_max)) + .then(pl.lit(f"Value is above hard maximum {hard_max}")) .when((soft_min is not None) & (pl.col(col) < soft_min)) .then(pl.lit(f"Value is below soft minimum {soft_min}")) .when((soft_max is not None) & (pl.col(col) > soft_max)) .then(pl.lit(f"Value is above soft maximum {soft_max}")) - .when((hard_max is not None) & (pl.col(col) > hard_max)) - .then(pl.lit(f"Value is above hard maximum {hard_max}")) ) col_df = safe_to_numeric(col_df, col) diff --git a/src/datasure/checks/outliers/report_ui.py b/src/datasure/checks/outliers/report_ui.py index aa6023af..bfc9973a 100644 --- a/src/datasure/checks/outliers/report_ui.py +++ b/src/datasure/checks/outliers/report_ui.py @@ -1,6 +1,9 @@ """Report-rendering UI for the outliers report.""" +import json from collections.abc import Callable +from dataclasses import dataclass, field, replace +from typing import Any import polars as pl import streamlit as st @@ -29,8 +32,35 @@ OutlierThresholds, SearchType, ) +from datasure.checks.outliers.review import ( + CONSTRAINTS, + OUTLIERS, + REVIEW_COLUMNS, + REVIEW_STATUS_COL, + VIOLATION_TYPE_COL, + FlagCheck, + FlagSelection, + TableFilters, + allowed_actions, + clear_reviewed_flags, + filter_table, + highlight_reviewed_row, + join_survey_columns, + key_has_conflicting_values, + mark_reviewed, + needs_hard_confirmation, + select_flag, +) from datasure.checks.outliers.settings_ui import outliers_report_settings -from datasure.utils.dataframe_utils import ColumnByType, sanitize_df_for_join +from datasure.processing.correction_log import HARD_SEVERITY +from datasure.processing.corrections import CorrectionProcessor +from datasure.utils.correction_form import ( + apply_correction_entries, + get_current_value, + render_correction_inputs, + should_enable_apply_button, +) +from datasure.utils.dataframe_utils import ColumnByType from datasure.utils.duckdb_utils import duckdb_get_table, duckdb_save_table from datasure.utils.navigations_utils import demo_callout from datasure.utils.onboarding_utils import is_demo_project @@ -39,6 +69,12 @@ save_check_settings, trigger_save, ) +from datasure.utils.ui_utils import ( + queue_notice, + row_styler, + show_queued_notices, + styled_dataframe, +) # ============================================================================= # Streamlit UI - Metrics Display @@ -135,6 +171,289 @@ def _render_outlier_metrics( ) +# ============================================================================= +# Streamlit UI - Correcting and Accepting Flags +# ============================================================================= + +# `queue_notice` scope of the confirmation shown after the post-save rerun. +_NOTICE_SCOPE = "outliers_corrections" + +# First column of a results table: a button that opens the correction dialog. +REVIEW_BUTTON_COL = "_review" +REVIEW_BUTTON_LABEL = ":material/edit_note: Review" + + +@dataclass(frozen=True) +class ReviewContext: + """What the results tables need to correct or accept flagged values. + + One context is built per report run, so lookups shared by both tables + are cached on it. + """ + + processor: CorrectionProcessor + alias: str + _corrections: dict[str, pl.DataFrame] = field( + default_factory=dict, compare=False, repr=False + ) + + def active_corrections(self, key_col: str) -> pl.DataFrame: + """Return the active value corrections, looked up once per run.""" + if key_col not in self._corrections: + self._corrections[key_col] = self.processor.get_active_corrections( + self.alias, key_col + ) + return self._corrections[key_col] + + +def _with_review_status( + flags: pl.DataFrame, + settings: OutlierSettings, + check: FlagCheck, + review: ReviewContext | None, +) -> pl.DataFrame: + """Mark accepted flags and corrected values; a no-op if already marked.""" + if review is None or flags.is_empty() or REVIEW_STATUS_COL in flags.columns: + return flags + acceptances = review.processor.get_active_acceptances( + review.alias, check.check_type, settings.survey_key + ) + corrections = review.active_corrections(settings.survey_key) + return mark_reviewed(flags, acceptances, settings.survey_key, check, corrections) + + +def _render_table_toggles( + check: FlagCheck, review: ReviewContext | None +) -> TableFilters: + """Render the toggles above a results table and return their values. + + "Show reviewed" and "Show only reviewed" are only offered with + `review`. While "Show only reviewed" is on, the other two toggles are + disabled, since it overrides them. + """ + reviewed_only_key = f"{check.check_type}_reviewed_only" + # Read before rendering so the toggles to its left can be disabled. + reviewed_only_on = review is not None and bool( + st.session_state.get(reviewed_only_key, False) + ) + + tc1, tc2, tc3, _ = st.columns([0.22, 0.22, 0.22, 0.34]) + with tc1: + flagged_only_on = st.toggle( + "Show only flagged values", + key=f"{check.check_type}_flagged_only", + value=True, + disabled=reviewed_only_on, + help="Turn off to show every checked value, flagged or not.", + ) + if review is None: + return TableFilters(flagged_only=flagged_only_on) + + with tc2: + show_reviewed = st.toggle( + "Show reviewed", + key=f"{check.check_type}_show_reviewed", + disabled=reviewed_only_on, + help="Also show flags accepted as valid and corrected values, " + "highlighted green, with the reason.", + ) + with tc3: + reviewed_only = st.toggle( + "Show only reviewed", + key=reviewed_only_key, + help="Show only flags accepted as valid and corrected values.", + ) + return TableFilters( + flagged_only=flagged_only_on, + show_reviewed=show_reviewed, + reviewed_only=reviewed_only, + ) + + +def _render_flags_table( + table: pl.DataFrame, + data: pl.DataFrame, + settings: OutlierSettings, + check: FlagCheck, + review: ReviewContext | None, + **dataframe_kwargs: Any, +) -> None: + """Render a results table; with `review`, each row has a Review button. + + Clicking Review opens the correction form for that row in a dialog. + """ + if review is None: + st.dataframe(table, **dataframe_kwargs) + return + + click_key = f"{check.check_type}_flag_review_click" + # Survey fields can be added to the table, so avoid their names. + button_col = REVIEW_BUTTON_COL + while button_col in table.columns: + button_col = f"_{button_col}" + shown = table.select(pl.lit(REVIEW_BUTTON_LABEL).alias(button_col), pl.all()) + column_config = { + button_col: st.column_config.ButtonColumn( + "", + type="tertiary", + pinned=True, + key=click_key, + help="Correct the value or accept it as valid.", + ) + } + if REVIEW_STATUS_COL in shown.columns: + # "Show reviewed" is on: colour the reviewed flags green. + styled_dataframe( + row_styler(shown, highlight_reviewed_row), + column_config=column_config, + **dataframe_kwargs, + ) + else: + st.dataframe(shown, column_config=column_config, **dataframe_kwargs) + + # The click is only present during the rerun it triggers, so the dialog + # opens once per click; widgets inside the dialog rerun just the dialog. + click = st.session_state.get(click_key) + rows = [click["row"]] if click else [] + selection = select_flag(table, rows, settings.survey_key, check) + if selection is not None: + _flag_correction_dialog(data, settings, selection, review) + + +@st.dialog("Correct or accept flagged value", width="medium") +def _flag_correction_dialog( + data: pl.DataFrame, + settings: OutlierSettings, + selection: FlagSelection, + review: ReviewContext, +) -> None: + """Show the correction form for a flag in a dialog.""" + _render_flag_correction_form(data, settings, selection, review) + + +def _render_flag_correction_form( + data: pl.DataFrame, + settings: OutlierSettings, + selection: FlagSelection, + review: ReviewContext, +) -> None: + """Render the shared correction form for the selected flag. + + The form is prefilled with the flag's KEY and column and the value in + the data. Accepting a hard constraint violation needs an extra + confirmation and is logged with severity "hard". A successful save + reruns the page so the tables and metrics reflect it. + + A KEY shared by rows with different values of the column can't be + reviewed, since a correction would change every one of those rows. + """ + key_col = settings.survey_key + key_value = selection.key_value + + st.markdown(f"**{selection.column}** for KEY **{key_value}**") + if key_has_conflicting_values(data, key_col, key_value, selection.column): + st.warning( + f"KEY {key_value} is on more than one row, with different values " + f"of {selection.column}. Corrections and acceptances apply to " + "every row with the KEY, so this value can't be reviewed here. " + "Give each record a unique KEY in the source data first; the " + "Duplicates check lists duplicated KEYs." + ) + return + + current_value = get_current_value(data, key_col, key_value, selection.column) + survey_id_value = ( + get_current_value(data, key_col, key_value, settings.survey_id) + if settings.survey_id + else None + ) + # JSON keeps the parts distinct: KEY "a_1"/column "b" and KEY "a"/column + # "1_b" would collide if joined with underscores. + namespace = json.dumps([selection.check_type, str(key_value), selection.column]) + + if selection.reviewed: + st.info( + "This flag was accepted as valid. Remove the acceptance on the " + "Correct Data page to flag it again." + ) + + state = render_correction_inputs( + data, + key_col, + str(key_value), + key_namespace=namespace, + actions=allowed_actions(selection), + column=selection.column, + current_value=current_value, + check_type=selection.check_type, + survey_id_value=survey_id_value, + ) + + hard_accept = needs_hard_confirmation(selection, state.action) + confirmed = True + if hard_accept: + st.warning( + "This value breaks a hard constraint, a bound meant to be " + "absolute. The acceptance is highlighted in the Correction Log." + ) + confirmed = st.checkbox( + "I confirm this value is correct despite the hard constraint", + key=f"correction_hard_confirm_{namespace}", + ) + + apply_enabled = ( + should_enable_apply_button(state.action, state.reason, state.new_value) + and not state.validation_error + and confirmed + ) + if not st.button( + label="Apply", + key=f"correction_apply_{namespace}", + width="stretch", + disabled=not apply_enabled, + type="primary", + ): + return + + # The form holds KEY as text for display; validation and the data + # compare the KEY's native value (e.g. 7, not "7"). + entry = replace(state.to_entry(), key_value=key_value) + if hard_accept: + entry = replace(entry, severity=HARD_SEVERITY) + if not apply_correction_entries( + review.processor, + review.alias, + key_col, + [entry], + source=selection.check_type, + ): + return + + # A full rerun closes the dialog and refreshes the tables and metrics. + queue_notice( + _NOTICE_SCOPE, + "toast", + f"Saved {state.action} on {selection.column} for KEY {key_value}. " + "It is listed in the Correction Log on the Correct Data page.", + ) + st.rerun() + + +def _show_saved_toast() -> None: + """Show the confirmation queued by a save before the page reran.""" + if not show_queued_notices(_NOTICE_SCOPE): + return + # A markdown link in the toast would open a new browser session and lose + # the selected project; a page link navigates within this session. + corrections_page = st.session_state.get("st_corr_page") + if corrections_page is not None: + st.page_link( + corrections_page, + label="Open the Correction Log", + icon=":material/cleaning_services:", + ) + + # ============================================================================= # Streamlit UI - Table Display # ============================================================================= @@ -149,9 +468,9 @@ def _render_display_columns_expander( ) -> list[str]: """Render the "Show more columns" expander and return the selected columns. - Shared by ``_render_constraint_violations_table`` and ``_render_outlier_table``, - which both let users add extra context columns to a results table, persisting - the selection to the settings file under ``settings_key``. + Used by ``_render_constraint_violations_table`` to let users add extra + context columns to the results table, persisting the selection to the + settings file under ``settings_key``. Parameters ---------- @@ -196,6 +515,7 @@ def _render_constraint_violations_table( violation_data: pl.DataFrame, settings: OutlierSettings, setting_file: str, + review: ReviewContext | None = None, ) -> None: """Render constraint violations table using Streamlit. @@ -209,11 +529,20 @@ def _render_constraint_violations_table( Outlier settings configuration. setting_file : str Path to settings file. + review : ReviewContext | None + If given, accepted violations are hidden (unless "Show reviewed" is + on) and selecting a row opens the correction form. """ if violation_data.is_empty(): st.info("No constraint violations detected.") return + violation_data = filter_table( + _with_review_status(violation_data, settings, CONSTRAINTS, review), + _render_table_toggles(CONSTRAINTS, review), + CONSTRAINTS, + ) + all_columns = data.columns include_cols = _build_include_cols( @@ -239,25 +568,20 @@ def _render_constraint_violations_table( # select columns to display from data display_df = data.select(include_cols) - # sanitize violation_data to avoid column name conflicts - violation_df = sanitize_df_for_join( - main_df=display_df, - join_df=violation_data, - join_key=settings.survey_key, - ) - - display_df = display_df.join( - violation_df, - on=settings.survey_key, - how="inner", + violations_df = join_survey_columns( + display_df, + violation_data, + settings.survey_key, + CONSTRAINTS, + reserved=[VIOLATION_TYPE_COL], ) - # show only rows with violations - violations_df = display_df.filter(pl.col("violation reason") != "no violation") - # add violation type column ie. "Soft Min", "Soft Max", "Hard Min", "Hard Max" + # (null for values within bounds, shown when flagged-only is off) violation_type_expr = ( - pl.when(pl.col("violation reason").str.contains("below hard minimum")) + pl.when(pl.col("violation reason") == CONSTRAINTS.no_flag) + .then(pl.lit(None, dtype=pl.String)) + .when(pl.col("violation reason").str.contains("below hard minimum")) .then(pl.lit("Hard Min")) .when(pl.col("violation reason").str.contains("below soft minimum")) .then(pl.lit("Soft Min")) @@ -269,71 +593,10 @@ def _render_constraint_violations_table( ) violations_df = violations_df.with_columns( - violation_type_expr.alias("violation type") - ) - - st.dataframe(violations_df) - - -def _render_outlier_table( - data: pl.DataFrame, - outliers_data: pl.DataFrame, - settings: OutlierSettings, - setting_file: str, -) -> None: - """Render outlier data table using Streamlit. - - Parameters - ---------- - data : pl.DataFrame - Original survey data. - outliers_data : pl.DataFrame - DataFrame containing outlier data. - settings : OutlierSettings - Outlier settings configuration. - setting_file : str - Path to settings file. - """ - if outliers_data.is_empty(): - st.info("No outliers detected in the selected columns.") - return - - all_columns = data.columns - - include_cols = _build_include_cols( - survey_key=settings.survey_key, - survey_id=settings.survey_id, - survey_date=settings.survey_date, - enumerator=settings.enumerator, - team=settings.team, - ) - - display_options = [col for col in all_columns if col not in include_cols] - - outlier_display_cols = _render_display_columns_expander( - setting_file, - "outlier_display_cols", - "outlier_display_cols", - display_options, - "Select additional columns to include in the outlier report.", + violation_type_expr.alias(VIOLATION_TYPE_COL) ) - if outlier_display_cols: - include_cols.extend(outlier_display_cols) - - # select columns to display from data - display_df = data.select(include_cols) - outliers_df = sanitize_df_for_join(display_df, outliers_data, settings.survey_key) - display_df = display_df.join( - outliers_df, - on=settings.survey_key, - how="inner", - ) - - # show only rows with outliers - outlier_show_df = display_df.filter(pl.col("outlier reason") != "no outlier") - - st.dataframe(outlier_show_df) + _render_flags_table(violations_df, data, settings, CONSTRAINTS, review) def _render_outlier_column_inspection( @@ -341,6 +604,7 @@ def _render_outlier_column_inspection( outliers_data: pl.DataFrame, settings: OutlierSettings, setting_file: str, + review: ReviewContext | None = None, ) -> None: """Inspect outlier columns in the DataFrame. @@ -354,6 +618,9 @@ def _render_outlier_column_inspection( Outlier settings configuration. setting_file : str Path to settings file. + review : ReviewContext | None + If given, accepted outliers are hidden (unless "Show reviewed" is + on) and selecting a row opens the correction form. """ if outliers_data.is_empty(): st.info( @@ -447,19 +714,26 @@ def _render_outlier_column_inspection( if inspect_display_cols: include_cols.extend(inspect_display_cols) + outliers_data = filter_table( + _with_review_status(outliers_data, settings, OUTLIERS, review), + _render_table_toggles(OUTLIERS, review), + OUTLIERS, + ) + # select columns to display from data display_df = data.select(include_cols) - outliers_df = sanitize_df_for_join(display_df, outliers_data, settings.survey_key) - display_df = display_df.join( - outliers_df, - on=settings.survey_key, - how="inner", + display_df = join_survey_columns( + display_df, outliers_data, settings.survey_key, OUTLIERS ) - st.dataframe( + _render_flags_table( display_df, + data, + settings, + OUTLIERS, + review, width="stretch", - hide_index=False, + hide_index=True, ) @@ -1200,6 +1474,7 @@ def outliers_report( setting_file: str, config: dict, survey_columns: ColumnByType, + alias: str | None = None, ) -> None: """Create a comprehensive outliers report. @@ -1215,13 +1490,22 @@ def outliers_report( Path to settings file. config : dict Configuration dictionary. + survey_columns : ColumnByType + Columns of `data` by type. + alias : str | None + The survey dataset alias. If given, flagged values can be corrected + or accepted from the results tables, and accepted flags are hidden + and left out of the metrics. """ + review = ReviewContext(CorrectionProcessor(project_id), alias) if alias else None + # get column info categorical_columns = survey_columns.categorical_columns datetime_columns = survey_columns.datetime_columns numeric_columns = survey_columns.numeric_columns st.title("Outliers and Constraints Report") + _show_saved_toast() if is_demo_project(): demo_callout( @@ -1240,6 +1524,14 @@ def outliers_report( outliers_settings = outliers_report_settings( setting_file, config_settings, categorical_columns, datetime_columns ) + if review is not None and outliers_settings.survey_key in REVIEW_COLUMNS: + st.warning( + f"Flags can't be corrected or accepted on this page because the " + f"Survey KEY column is named '{outliers_settings.survey_key}', " + "which this report uses for review results. Rename the column to " + "review flags here." + ) + review = None # Outlier columns configuration st.subheader("Outlier/Constraint Columns Configuration") @@ -1314,8 +1606,13 @@ def outliers_report( st.info("No constraint violations detected.") else: - # show constraint metrics - _render_constraint_metrics(constraint_violations) + constraint_violations = _with_review_status( + constraint_violations, outliers_settings, CONSTRAINTS, review + ) + # show constraint metrics, leaving out accepted violations + _render_constraint_metrics( + clear_reviewed_flags(constraint_violations, CONSTRAINTS) + ) # show constraint violations table st.subheader("Constraint Violations Details") @@ -1324,6 +1621,7 @@ def outliers_report( constraint_violations, outliers_settings, setting_file, + review=review, ) # show outliers metrics @@ -1355,8 +1653,13 @@ def outliers_report( st.info("No outliers detected.") else: - # show outlier metrics - _render_outlier_metrics(outlier_data, outliers_settings) + outlier_data = _with_review_status( + outlier_data, outliers_settings, OUTLIERS, review + ) + # show outlier metrics, leaving out accepted outliers + _render_outlier_metrics( + clear_reviewed_flags(outlier_data, OUTLIERS), outliers_settings + ) # show outlier column inspection st.subheader("Inspect Columns") @@ -1366,6 +1669,7 @@ def outliers_report( outlier_data, outliers_settings, setting_file, + review=review, ) demo_callout( diff --git a/src/datasure/checks/outliers/review.py b/src/datasure/checks/outliers/review.py new file mode 100644 index 00000000..e08b5443 --- /dev/null +++ b/src/datasure/checks/outliers/review.py @@ -0,0 +1,406 @@ +"""Review of outlier and constraint flags against the correction log. + +A flag is reviewed when the correction log holds an active acceptance for +its KEY and column under the same check type (see +`CorrectionProcessor.get_active_acceptances`). Reviewed flags are hidden from +the tables unless the user asks to see them, and are left out of the flag +counts. Outlier and constraint acceptances are independent: accepting an +outlier does not review a constraint violation on the same cell. + +A cell whose current value comes from a correction (see +`CorrectionProcessor.get_active_corrections`) is marked corrected. Corrected +values are shown with the reviewed ones, but a corrected value that is still +flagged stays visible and counted: it still needs attention. + +Kept free of Streamlit so the logic can be tested without a running app. +""" + +from collections.abc import Sequence +from dataclasses import dataclass +from typing import Any + +import polars as pl + +from datasure.processing.correction_log import HARD_SEVERITY, Action + +REVIEW_STATUS_COL = "review status" +REVIEW_REASON_COL = "review reason" +# The columns `mark_reviewed` adds to the results. +REVIEW_COLUMNS = (REVIEW_STATUS_COL, REVIEW_REASON_COL) +REVIEWED_BADGE = "Reviewed" +CORRECTED_BADGE = "Corrected" + + +@dataclass(frozen=True) +class FlagCheck: + """How one check reports its flags in the computed results.""" + + check_type: str + reason_col: str + no_flag: str + + +OUTLIERS = FlagCheck("outliers", "outlier reason", "no outlier") +CONSTRAINTS = FlagCheck("constraints", "violation reason", "no violation") + +# Flag columns of the computed results that the review logic reads. +COLUMN_NAME_COL = "column name" +# Added to the constraint table by `_render_constraint_violations_table`. +VIOLATION_TYPE_COL = "violation type" + +# Violation types (see `_render_constraint_violations_table`) of hard bounds. +_HARD_VIOLATION_TYPES = ("Hard Min", "Hard Max") + + +@dataclass(frozen=True) +class FlagSelection: + """The flag behind a selected table row, used to prefill the form.""" + + key_value: Any + column: str + check_type: str + flagged: bool + reviewed: bool + hard: bool + + +def _is_flagged(check: FlagCheck) -> pl.Expr: + return pl.col(check.reason_col).is_not_null() & ( + pl.col(check.reason_col) != check.no_flag + ) + + +def _latest_entry_by_cell(log_rows: pl.DataFrame, prefix: str) -> pl.DataFrame: + """Return the latest log reason and severity per KEY and column, for a join. + + The log stores KEY as text, so keys are compared as text. Logs without + a severity column have a null severity. + """ + severity = ( + pl.col("severity").cast(pl.String) + if "severity" in log_rows.columns + else pl.lit(None, dtype=pl.String) + ) + return log_rows.select( + pl.col("KEY").cast(pl.String).alias("_key"), + pl.col("column").cast(pl.String).alias("_column"), + pl.col("reason").cast(pl.String).alias(f"_{prefix}_reason"), + severity.alias(f"_{prefix}_severity"), + ).unique(subset=["_key", "_column"], keep="last") + + +def _is_hard_violation(check: FlagCheck) -> pl.Expr: + """Whether a row's flag is a hard constraint violation. + + Matches the reasons written by `compute_constraint_violations`. + """ + return ( + pl.col(check.reason_col) + .fill_null("") + .str.contains("below hard minimum|above hard maximum") + ) + + +def mark_reviewed( + flags: pl.DataFrame, + acceptances: pl.DataFrame, + survey_key: str, + check: FlagCheck, + corrections: pl.DataFrame | None = None, +) -> pl.DataFrame: + """Add review status and reason columns to computed flags. + + Parameters + ---------- + flags : pl.DataFrame + Output of `compute_outlier_output` or `compute_constraint_violations`: + one row per KEY and column name. + acceptances : pl.DataFrame + The active acceptances for `check`, as returned by + `CorrectionProcessor.get_active_acceptances`. + survey_key : str + The Survey KEY column in `flags`. + check : FlagCheck + The check that produced `flags`. + corrections : pl.DataFrame | None + The active value corrections, as returned by + `CorrectionProcessor.get_active_corrections`. + + Returns + ------- + pl.DataFrame + `flags` in the same order, plus `REVIEW_STATUS_COL` and + `REVIEW_REASON_COL`: `REVIEWED_BADGE` and the acceptance reason for + an accepted flag (only flagged rows can be accepted), + `CORRECTED_BADGE` and the correction reason for a corrected cell, + otherwise null. An acceptance takes precedence over a correction. + + Raises + ------ + ValueError + If `survey_key` is one of `REVIEW_COLUMNS`, which would overwrite it. + """ + if flags.is_empty(): + return flags + if survey_key in REVIEW_COLUMNS: + raise ValueError( + f"The Survey KEY column '{survey_key}' has the name of a review column" + ) + + if corrections is None: + corrections = acceptances.clear() + + # Match in a frame of our own columns, so the helper columns can't clash + # with a survey KEY or field of the same name. + cells = flags.select( + pl.col(survey_key).cast(pl.String).alias("_key"), + pl.col(COLUMN_NAME_COL).cast(pl.String).alias("_column"), + pl.col(check.reason_col), + ) + for prefix, log_rows in (("accept", acceptances), ("correct", corrections)): + cells = cells.join( + _latest_entry_by_cell(log_rows, prefix), + on=["_key", "_column"], + how="left", + maintain_order="left", + ) + + # An acceptance covers a hard violation only if it was confirmed as one: + # a value accepted as a soft violation can become hard when bounds are + # tightened, and must then be confirmed again. + accepted = ( + _is_flagged(check) + & pl.col("_accept_reason").is_not_null() + & ( + ~_is_hard_violation(check) + | (pl.col("_accept_severity").fill_null("") == HARD_SEVERITY) + ) + ) + corrected = pl.col("_correct_reason").is_not_null() + # Each cell matches at most one log row, so `cells` lines up with `flags`. + review = cells.select( + pl.when(accepted) + .then(pl.lit(REVIEWED_BADGE)) + .when(corrected) + .then(pl.lit(CORRECTED_BADGE)) + .alias(REVIEW_STATUS_COL), + pl.when(accepted) + .then(pl.col("_accept_reason")) + .when(corrected) + .then(pl.col("_correct_reason")) + .alias(REVIEW_REASON_COL), + ) + return flags.with_columns(review.get_columns()) + + +def _is_reviewed() -> pl.Expr: + """Whether a row is an accepted flag (not merely a corrected value).""" + return pl.col(REVIEW_STATUS_COL).fill_null("") == REVIEWED_BADGE + + +def clear_reviewed_flags(flags: pl.DataFrame, check: FlagCheck) -> pl.DataFrame: + """Return `flags` with reviewed flags reported as unflagged, for metrics. + + Rows are kept, so the number of columns checked is unchanged. + """ + if REVIEW_STATUS_COL not in flags.columns: + return flags + return flags.with_columns( + pl.when(_is_reviewed()) + .then(pl.lit(check.no_flag)) + .otherwise(pl.col(check.reason_col)) + .alias(check.reason_col) + ) + + +_REVIEWED_ROW_STYLE = "background-color: rgba(25, 135, 84, 0.15)" + + +def highlight_reviewed_row(row: Any) -> list[str]: + """Style every cell of an accepted or corrected row green in a table. + + Used with a pandas ``Styler`` (``df.style.apply(highlight_reviewed_row, + axis=1)``) when "Show reviewed" is on. + """ + status = row.get(REVIEW_STATUS_COL) + # Missing values may be pd.NA, which can't be used in a boolean test. + is_reviewed = isinstance(status, str) and status in ( + REVIEWED_BADGE, + CORRECTED_BADGE, + ) + return [_REVIEWED_ROW_STYLE if is_reviewed else ""] * len(row) + + +def flagged_only(flags: pl.DataFrame, check: FlagCheck) -> pl.DataFrame: + """Return the rows of `flags` that `check` flagged.""" + if check.reason_col not in flags.columns: + return flags + return flags.filter(_is_flagged(check)) + + +def visible_flags(flags: pl.DataFrame, *, show_reviewed: bool) -> pl.DataFrame: + """Return the rows and columns of `flags` to show in a results table. + + Reviewed flags and the review columns are hidden unless `show_reviewed`. + """ + if REVIEW_STATUS_COL not in flags.columns: + return flags + if show_reviewed: + return flags + return flags.filter(~_is_reviewed()).drop(REVIEW_STATUS_COL, REVIEW_REASON_COL) + + +@dataclass(frozen=True) +class TableFilters: + """The toggles above a results table. + + Attributes + ---------- + flagged_only : bool + Show only flagged values. + show_reviewed : bool + Also show accepted flags, with the review columns. + reviewed_only : bool + Show only accepted and corrected rows, whatever the other toggles. + """ + + flagged_only: bool = True + show_reviewed: bool = False + reviewed_only: bool = False + + +def filter_table( + flags: pl.DataFrame, filters: TableFilters, check: FlagCheck +) -> pl.DataFrame: + """Return the rows and columns of marked `flags` that `filters` show.""" + if filters.reviewed_only: + # A value corrected into range is unflagged, so flagged-only and + # show-reviewed are ignored here. + if REVIEW_STATUS_COL not in flags.columns: + return flags.clear() + return flags.filter(pl.col(REVIEW_STATUS_COL).is_not_null()) + + flags = visible_flags(flags, show_reviewed=filters.show_reviewed) + if filters.flagged_only: + flags = flagged_only(flags, check) + return flags + + +SURVEY_COL_SUFFIX = " (survey)" + + +def join_survey_columns( + survey: pl.DataFrame, + flags: pl.DataFrame, + survey_key: str, + check: FlagCheck, + *, + reserved: Sequence[str] = (), +) -> pl.DataFrame: + """Join survey display columns onto `flags` for a results table. + + The flag columns stay authoritative: a survey column named like one of + them, like a review column (`REVIEW_COLUMNS`, even when hidden), or like + a `reserved` column added afterwards, is renamed with + `SURVEY_COL_SUFFIX`. Otherwise a survey field called "column name" + would become the correction target, and one called "review status" + would be read as the flag's review state. + + The result is sorted by KEY, column name and flag reason, so a row + position reported by a Review click resolves to the same flag on the + rerun it triggers whatever order the join returns. + """ + generated = set(flags.columns) | set(REVIEW_COLUMNS) | set(reserved) + taken = generated | set(survey.columns) + renames = {} + for col in survey.columns: + if col == survey_key or col not in generated: + continue + new_name = f"{col}{SURVEY_COL_SUFFIX}" + while new_name in taken: + new_name = f"{new_name}{SURVEY_COL_SUFFIX}" + renames[col] = new_name + taken.add(new_name) + + joined = survey.rename(renames).join(flags, on=survey_key, how="inner") + sort_cols = [ + col + for col in (survey_key, COLUMN_NAME_COL, check.reason_col) + if col in joined.columns + ] + return joined.sort(sort_cols, nulls_last=True, maintain_order=True) + + +def select_flag( + table: pl.DataFrame, + rows: list[int], + survey_key: str, + check: FlagCheck, +) -> FlagSelection | None: + """Return the flag behind the selected row of a results table. + + Parameters + ---------- + table : pl.DataFrame + The table as displayed. + rows : list[int] + Selected row positions, as reported by `st.dataframe` selection. + survey_key : str + The Survey KEY column in `table`. + check : FlagCheck + The check the table shows. + + Returns + ------- + FlagSelection | None + The selection, or None if no row is selected or the position no + longer exists in `table`. + """ + if not rows or not 0 <= rows[0] < table.height: + return None + + row = table.row(rows[0], named=True) + flagged = check.reason_col in table.columns and bool( + table.slice(rows[0], 1).select(_is_flagged(check)).item() + ) + return FlagSelection( + key_value=row[survey_key], + column=row[COLUMN_NAME_COL], + check_type=check.check_type, + flagged=flagged, + reviewed=row.get(REVIEW_STATUS_COL) == REVIEWED_BADGE, + hard=row.get(VIOLATION_TYPE_COL) in _HARD_VIOLATION_TYPES, + ) + + +def key_has_conflicting_values( + data: pl.DataFrame, survey_key: str, key_value: Any, column: str +) -> bool: + """Whether rows sharing `key_value` hold different values of `column`. + + Corrections and acceptances target a KEY, not a row: a modification + changes every row with the KEY, and an acceptance holds only while every + row has the accepted value. A flag on such a KEY can't be reviewed + without changing or misreading another record. + """ + if survey_key not in data.columns or column not in data.columns: + return False + values = data.filter(pl.col(survey_key) == key_value).get_column(column) + return values.n_unique() > 1 + + +def allowed_actions(selection: FlagSelection) -> list[Action]: + """Return the actions the correction form offers for `selection`. + + A value can be accepted only while it is flagged and not yet reviewed. + Rows are removed from the Corrections page, not from a check page. + """ + actions = [Action.MODIFY_VALUE, Action.REMOVE_VALUE] + if selection.flagged and not selection.reviewed: + actions.append(Action.ACCEPT) + return actions + + +def needs_hard_confirmation(selection: FlagSelection, action: Action) -> bool: + """Whether applying `action` needs the hard-violation confirmation step.""" + return action == Action.ACCEPT and selection.hard diff --git a/src/datasure/checks/progress.py b/src/datasure/checks/progress.py index 6a31d8e9..799b8fd9 100644 --- a/src/datasure/checks/progress.py +++ b/src/datasure/checks/progress.py @@ -10,7 +10,6 @@ from typing import Any, Literal -import pandas as pd import plotly.graph_objects as go import polars as pl import seaborn as sns @@ -26,11 +25,12 @@ save_check_settings, trigger_save, ) +from datasure.utils.ui_utils import ensure_styler_limit TAB_NAME = "progress" -# Configure pandas styler for large dataframes (performance optimization) -pd.set_option("styler.render.max_elements", 1_000_000) +# Allow styling large dataframes (the limit is shared and only raised) +ensure_styler_limit(1_000_000) # ============================================================================= @@ -1318,8 +1318,8 @@ def _display_chart_and_table( with ai2: # Convert to pandas for styling (Streamlit doesn't support Polars styling yet) attempts_pd = attempted_interviews.to_pandas() - # Dynamically set pd styler max elements based on DataFrame size - pd.set_option("styler.render.max_elements", attempts_pd.size + 1) + # Make sure the Styler limit fits this DataFrame + ensure_styler_limit(attempts_pd.size + 1) cmap = sns.light_palette("pink", as_cmap=True) vmin = attempts_pd["num_interviews"].min() diff --git a/src/datasure/checks/summary.py b/src/datasure/checks/summary.py index 97d1c1c8..da52770b 100644 --- a/src/datasure/checks/summary.py +++ b/src/datasure/checks/summary.py @@ -30,6 +30,7 @@ save_check_settings, trigger_save, ) +from datasure.utils.ui_utils import ensure_styler_limit TAB_NAME: str = "summary" @@ -1312,7 +1313,7 @@ def _render_progress_by_column( # Display heatmap cmap = sns.light_palette("pink", as_cmap=True) styler_limit = progress_data.shape[0] * progress_data.shape[1] - pd.set_option("styler.render.max_elements", styler_limit) + ensure_styler_limit(styler_limit) st.dataframe( progress_data.style.format(subset=format_cols, precision=0).background_gradient( subset=format_cols, cmap=cmap, axis=1, vmin=vmin_val, vmax=vmax_val diff --git a/src/datasure/processing/correction_log.py b/src/datasure/processing/correction_log.py index 81b54f25..dd42f56c 100644 --- a/src/datasure/processing/correction_log.py +++ b/src/datasure/processing/correction_log.py @@ -35,6 +35,9 @@ class Action(StrEnum): ACCEPT_CHECK_TYPES = ("outliers", "constraints", "backchecks", "duplicates", "gps") +# `severity` of an acceptance that overrides a hard constraint bound. +HARD_SEVERITY = "hard" + # Full schema of a persisted correction log (`corr_log_{alias}`), in column order. CORRECTION_LOG_SCHEMA: dict[str, pl.DataType] = { "date": pl.Datetime("us"), @@ -49,6 +52,9 @@ class Action(StrEnum): "status_reason": pl.String, "source": pl.String, "check_type": pl.String, + # For "accept", how serious the accepted flag is: "hard" for a hard + # constraint violation, null otherwise. + "severity": pl.String, } # Values given to columns that were added to the log after some logs were @@ -59,6 +65,7 @@ class Action(StrEnum): "status_reason": None, "source": CORRECTIONS_PAGE_SOURCE, "check_type": None, + "severity": None, } diff --git a/src/datasure/processing/corrections.py b/src/datasure/processing/corrections.py index e50e0bf2..27447d48 100644 --- a/src/datasure/processing/corrections.py +++ b/src/datasure/processing/corrections.py @@ -13,6 +13,7 @@ CORRECTION_ACTIONS, CORRECTION_LOG_SCHEMA, CORRECTIONS_PAGE_SOURCE, + HARD_SEVERITY, Action, empty_correction_log, ensure_log_columns, @@ -138,6 +139,26 @@ def _acceptance_is_active( ) +def _stored_correction_value(data: pl.DataFrame, row: dict[str, Any]) -> str | None: + """Return the value a logged value correction left in the data, as logged. + + "remove value" leaves the cell missing. "modify value" stores its new + value cast to the column's type, as `_apply_modify_value` does, so a + Float32 column holds 70.0999984741211 for a logged "70.1". If the cast + fails, the logged text is returned unchanged. + """ + if row["action"] != Action.MODIFY_VALUE: + return None + new_value, column = row["new_value"], row["column"] + if new_value is None or column not in data.columns: + return new_value + try: + typed = pl.select(pl.lit(new_value).cast(data.schema[column])).item() + except pl.exceptions.PolarsError: + return new_value + return _encode_scalar(typed) + + def _check_acceptance_against_data( data: pl.DataFrame, key_col: str, @@ -159,14 +180,28 @@ def _check_acceptance_against_data( def _validate_acceptance( - check_type: str | None, column: str | None, current_value: Any + check_type: str | None, + column: str | None, + current_value: Any, + severity: str | None = None, ) -> None: - """Raise ValueError if an acceptance's check type, column and value don't fit.""" + """Raise ValueError if an acceptance's check type, column, value and + severity don't fit. + """ if check_type not in ACCEPT_CHECK_TYPES: raise ValueError( f"Unknown check type '{check_type}'; expected one of " f"{', '.join(ACCEPT_CHECK_TYPES)}" ) + # The Correction Log highlights hard acceptances as hard-constraint + # overrides, so only a constraint acceptance may be hard. + if severity is not None and ( + severity != HARD_SEVERITY or check_type != "constraints" + ): + raise ValueError( + f"Severity '{severity}' is not valid for a {check_type} acceptance; " + f"only constraint acceptances can have severity '{HARD_SEVERITY}'" + ) if check_type == "gps": if ( column is not None @@ -191,6 +226,7 @@ def _build_log_row( reason: str, source: str, check_type: str | None, + severity: str | None = None, ) -> dict[str, Any]: """Build one correction-log row. @@ -210,6 +246,7 @@ def _build_log_row( "status_reason": None, "source": str(source), "check_type": check_type, + "severity": severity, } @@ -237,6 +274,9 @@ class CorrectionEntry: The Survey ID value for this KEY, recorded in the log's ID column check_type : str | None For "accept", the check whose flag is accepted + severity : str | None + For "accept", how serious the accepted flag is: "hard" for a hard + constraint violation, otherwise None """ key_value: str @@ -247,6 +287,7 @@ class CorrectionEntry: new_value: Any = None survey_id_value: Any = None check_type: str | None = None + severity: str | None = None # The cached methods below hash `self` by its project so that two projects @@ -545,6 +586,61 @@ def get_active_acceptances( ] return acceptances.filter(pl.Series(is_active, dtype=pl.Boolean)) + def get_active_corrections(self, alias: str, key_col: str) -> pl.DataFrame: + """Return the value corrections whose result the data still holds. + + A "modify value" is active while the cell holds its new value, and a + "remove value" while the cell is missing. A correction overwritten by + a later one, whose row was removed, or that failed to reapply to the + current prep data, is inactive. + + Parameters + ---------- + alias : str + The data alias/table name + key_col : str + The Survey KEY column name + + Returns + ------- + pl.DataFrame + The active "modify value" and "remove value" rows from the + correction log, in log order + """ + log = self.get_correction_log(alias) + if log.width == 0: + return empty_correction_log() + + # A correction that failed to reapply was not applied, even if the + # prep data happens to hold its new value. + corrections = log.filter( + pl.col("action").is_in([Action.MODIFY_VALUE, Action.REMOVE_VALUE]) + & pl.col("column").is_not_null() + & (pl.col("status") == "Successful") + ) + if corrections.is_empty(): + return corrections + + data = self.get_corrected_data(alias) + if key_col in data.columns: + # Check only the rows of logged KEYs, not the whole dataset per + # correction. Every row of a duplicated KEY is kept, so all of + # them must still hold the value. + logged_keys = corrections["KEY"].unique() + data = data.filter(pl.col(key_col).cast(pl.String).is_in(logged_keys)) + + is_active = [ + _acceptance_mismatch( + data, + key_col, + row["KEY"], + {row["column"]: _stored_correction_value(data, row)}, + ) + is None + for row in corrections.iter_rows(named=True) + ] + return corrections.filter(pl.Series(is_active, dtype=pl.Boolean)) + def apply_correction( self, alias: str, @@ -663,6 +759,7 @@ def apply_corrections( reason=entry.reason, source=source, check_type=entry.check_type, + severity=entry.severity, ) for entry in entries ] @@ -692,12 +789,20 @@ def _apply_entry( ) if entry.action == Action.ACCEPT: - _validate_acceptance(entry.check_type, entry.column, entry.current_value) + _validate_acceptance( + entry.check_type, entry.column, entry.current_value, entry.severity + ) _check_acceptance_against_data( data, key_col, entry.key_value, entry.column, entry.current_value ) return data + if entry.severity is not None: + raise ValueError( + f"Only acceptances record a severity, not {entry.action} " + f"on {entry.key_value}" + ) + if entry.action not in CORRECTION_ACTIONS: raise ValueError(f"Unknown correction action '{entry.action}'") @@ -1252,6 +1357,7 @@ def get_correction_summary(self, alias: str) -> list[dict[str, Any]]: "action_index": f"{index} - {action} - {description}", "action": action, "check_type": row["check_type"], + "severity": row["severity"], "description": description, "key_value": key_value, "column": column, diff --git a/src/datasure/utils/reapply_utils.py b/src/datasure/utils/reapply_utils.py index b3236649..81bff109 100644 --- a/src/datasure/utils/reapply_utils.py +++ b/src/datasure/utils/reapply_utils.py @@ -20,12 +20,15 @@ class ReapplyFailure: reason: str -def highlight_status(value: str) -> str: +def highlight_status(value: object) -> str: """Style a log status cell: green text for Successful, red for Failed. Used with a pandas ``Styler`` (``df.style.map(highlight_status, subset=["status"])``) on the prep and correction Change Log tables. """ + if not isinstance(value, str): + # Missing (None or pd.NA, which can't be used in a boolean test). + return "" if value == "Failed": return "color: #dc3545; font-weight: 600" if value == "Successful": diff --git a/src/datasure/utils/ui_utils.py b/src/datasure/utils/ui_utils.py index 33c08ab8..bdcbfcac 100644 --- a/src/datasure/utils/ui_utils.py +++ b/src/datasure/utils/ui_utils.py @@ -12,11 +12,16 @@ swap regardless of the order in which the module was first imported. """ +import threading from collections.abc import Callable, Sequence from dataclasses import dataclass -from typing import Literal +from typing import TYPE_CHECKING, Any, Literal -NoticeLevel = Literal["success", "warning", "error"] +if TYPE_CHECKING: + import polars as pl + from pandas.io.formats.style import Styler + +NoticeLevel = Literal["success", "warning", "error", "toast"] _QUEUED_NOTICES_KEY = "st_queued_notices" @@ -174,8 +179,9 @@ def queue_notice(scope: str, level: NoticeLevel, message: str) -> None: ---------- scope : str Where the message belongs, e.g. ``"prep_survey"`` for one Prep tab. - level : {"success", "warning", "error"} - The Streamlit callout used to render the message. + level : {"success", "warning", "error", "toast"} + The Streamlit callout used to render the message, or "toast" for a + transient ``st.toast``. message : str The message text (Markdown). """ @@ -185,10 +191,63 @@ def queue_notice(scope: str, level: NoticeLevel, message: str) -> None: queued.setdefault(scope, []).append(Notice(level, message)) -def show_queued_notices(scope: str) -> None: - """Render and clear the messages queued for a scope, in queue order.""" +def show_queued_notices(scope: str) -> bool: + """Render and clear the messages queued for a scope, in queue order. + + Returns True if any message was shown. + """ import streamlit as st queued = st.session_state.get(_QUEUED_NOTICES_KEY, {}) - for notice in queued.pop(scope, []): + notices = queued.pop(scope, []) + for notice in notices: getattr(st, notice.level)(notice.message) + return bool(notices) + + +# Serializes updates to pandas' process-wide Styler limit across sessions. +_STYLER_LIMIT_LOCK = threading.Lock() + + +def ensure_styler_limit(cells: int) -> None: + """Raise pandas' ``styler.render.max_elements`` to at least `cells`. + + Streamlit refuses to render a Styler with more cells than this + process-wide option, and every session shares it. The limit is only + ever raised, never lowered or restored, so one session can't cut it + below what another's render needs. Use this instead of + ``pd.set_option("styler.render.max_elements", ...)``. + """ + import pandas as pd + + with _STYLER_LIMIT_LOCK: + if pd.get_option("styler.render.max_elements") < cells: + pd.set_option("styler.render.max_elements", cells) + + +def styled_dataframe(styler: "Styler", **dataframe_kwargs: Any) -> Any: + """Render a pandas ``Styler`` with ``st.dataframe``, whatever its size. + + Raises the Styler cell limit to fit the table first (see + `ensure_styler_limit`). Returns what ``st.dataframe`` returns. + """ + import streamlit as st + + ensure_styler_limit(styler.data.size) + return st.dataframe(styler, **dataframe_kwargs) + + +def row_styler(df: "pl.DataFrame", row_style: Callable[[Any], list[str]]) -> "Styler": + """Return a pandas ``Styler`` for `df` that styles each row with `row_style`. + + ``st.dataframe`` shows a Styler's formatted text, and pandas' defaults + would change the values: integers with missing values become floats, + floats are padded or rounded to six decimals, and missing values read + "nan". Here nullable types are kept and each value is shown as its plain + text ("None" if missing), so styling changes only the colours. + + `row_style` receives each row as a pandas Series; missing values are + ``pd.NA``, which must not be used in a boolean test. + """ + pandas_df = df.to_pandas(use_pyarrow_extension_array=True) + return pandas_df.style.apply(row_style, axis=1).format(str, na_rep="None") diff --git a/src/datasure/views/correction_view.py b/src/datasure/views/correction_view.py index d9c18b5f..12434e90 100644 --- a/src/datasure/views/correction_view.py +++ b/src/datasure/views/correction_view.py @@ -14,6 +14,7 @@ from datasure.processing.correction_log import ( CORRECTIONS_PAGE_SOURCE, + HARD_SEVERITY, Action, ensure_log_columns, ) @@ -38,7 +39,9 @@ confirm_dialog, metric_row, page_header, + row_styler, section_header, + styled_dataframe, ) @@ -536,8 +539,8 @@ def _build_correction_log_display(correction_log: pl.DataFrame) -> pl.DataFrame: Backfills columns missing from logs saved before they existed, orders columns so status/status_reason sit right after action, and relabels the "ID" column as "Survey ID" for display. "accept" rows carry the check - whose flag was accepted in check_type; source names the page that made - each entry. + whose flag was accepted in check_type and, for a hard constraint + violation, severity "hard"; source names the page that made each entry. Parameters ---------- @@ -560,6 +563,7 @@ def _build_correction_log_display(correction_log: pl.DataFrame) -> pl.DataFrame: "status", "status_reason", "check_type", + "severity", "column", "current_value", "new_value", @@ -571,6 +575,25 @@ def _build_correction_log_display(correction_log: pl.DataFrame) -> pl.DataFrame: return correction_log.select(display_columns).rename({"ID": "Survey ID"}) +def highlight_hard_acceptance(row: Any) -> list[str]: + """Style every cell of a hard-violation acceptance in the Correction Log. + + Used with a pandas ``Styler`` (``df.style.apply(highlight_hard_acceptance, + axis=1)``). Accepting a value that breaks a hard constraint overrides a + bound meant to be absolute, so those rows stand out for review. + """ + action, severity = row.get("action"), row.get("severity") + # Missing values may be pd.NA, which can't be used in a boolean test. + is_hard_accept = ( + isinstance(action, str) + and isinstance(severity, str) + and action == Action.ACCEPT + and severity == HARD_SEVERITY + ) + style = "background-color: rgba(220, 53, 69, 0.15)" if is_hard_accept else "" + return [style] * len(row) + + @st.fragment def render_correction_log( correction_processor: CorrectionProcessor, alias: str, tab_index: int @@ -598,9 +621,11 @@ def render_correction_log( else: section_header("Correction Log") - log_display = _build_correction_log_display(correction_log).to_pandas() - st.dataframe( - log_display.style.map(highlight_status, subset=["status"]), + log_display = _build_correction_log_display(correction_log) + styled_dataframe( + row_styler(log_display, highlight_hard_acceptance).map( + highlight_status, subset=["status"] + ), width="stretch", ) diff --git a/src/datasure/views/output_view_template.py b/src/datasure/views/output_view_template.py index 8f041223..1e628b36 100644 --- a/src/datasure/views/output_view_template.py +++ b/src/datasure/views/output_view_template.py @@ -369,6 +369,7 @@ def render_check_tabs(project_id: str, config: PageConfig, data: CheckData) -> N config.setting_file, outliers_config, survey_columns, + alias=config.survey_data_name, ) with gps_checks: diff --git a/tests/checks/outliers/test_compute.py b/tests/checks/outliers/test_compute.py index 2c96c0a9..ba63b475 100644 --- a/tests/checks/outliers/test_compute.py +++ b/tests/checks/outliers/test_compute.py @@ -329,6 +329,30 @@ def test_no_bounds_set(self, sample_polars_df, outlier_settings): ) assert result.is_empty() + def test_hard_bounds_take_precedence_over_soft_bounds(self, outlier_settings): + """A value past a hard bound is a hard violation, not a soft one.""" + data = pl.DataFrame( + {"survey_key": ["K1", "K2", "K3", "K4"], "age": [150, 70, -5, 10]} + ) + config = pl.DataFrame( + { + "column_name": [["age"]], + "hard_min": [0.0], + "soft_min": [15.0], + "soft_max": [65.0], + "hard_max": [100.0], + } + ) + + result = compute_constraint_violations(data, outlier_settings, config) + + assert result["violation reason"].to_list() == [ + "Value is above hard maximum 100.0", + "Value is above soft maximum 65.0", + "Value is below hard minimum 0.0", + "Value is below soft minimum 15.0", + ] + class TestComputeColumnOutlierSummary: """Test compute_column_outlier_summary function.""" diff --git a/tests/checks/outliers/test_report_ui.py b/tests/checks/outliers/test_report_ui.py index 71353b2c..11571b26 100644 --- a/tests/checks/outliers/test_report_ui.py +++ b/tests/checks/outliers/test_report_ui.py @@ -30,7 +30,6 @@ _render_outlier_metrics, _render_outlier_options, _render_outlier_settings_table, - _render_outlier_table, _render_search_type_selection, _update_outlier_column_config, _validate_constraint_settings, @@ -471,51 +470,6 @@ def test_non_empty_with_extra_display_cols( st_mock.dataframe.assert_called_once() -# ============================================================================ -# TESTS: _render_outlier_table -# ============================================================================ - - -class TestRenderOutlierTable: - """Test _render_outlier_table function.""" - - def test_empty_data_shows_info(self, base_survey_data, outlier_settings): - with patch("datasure.checks.outliers.report_ui.st") as st_mock: - _render_outlier_table( - base_survey_data, - pl.DataFrame(), - outlier_settings, - "settings.json", - ) - st_mock.info.assert_called_once() - - def test_non_empty_data_shows_dataframe(self, base_survey_data, outlier_settings): - outliers_data = pl.DataFrame( - { - "survey_key": ["K001"], - "column name": ["col1"], - "outlier reason": ["Value is above upper bound 50.00"], - } - ) - with ( - patch("datasure.checks.outliers.report_ui.st") as st_mock, - patch( - "datasure.checks.outliers.report_ui.load_check_settings", - return_value={}, - ), - patch("datasure.checks.outliers.report_ui.save_check_settings"), - ): - st_mock.columns.side_effect = _columns_side_effect - st_mock.multiselect.return_value = [] - _render_outlier_table( - base_survey_data, - outliers_data, - outlier_settings, - "settings.json", - ) - st_mock.dataframe.assert_called_once() - - # ============================================================================ # TESTS: _render_outlier_column_inspection # ============================================================================ diff --git a/tests/checks/outliers/test_report_ui_corrections.py b/tests/checks/outliers/test_report_ui_corrections.py new file mode 100644 index 00000000..8cb62e4e --- /dev/null +++ b/tests/checks/outliers/test_report_ui_corrections.py @@ -0,0 +1,923 @@ +"""Tests for correcting and accepting flags from the outliers report tables.""" + +from unittest.mock import MagicMock, patch + +import polars as pl +import pytest +from pandas.io.formats.style import Styler + +from datasure.checks.outliers.models import OutlierSettings +from datasure.checks.outliers.report_ui import ( + REVIEW_BUTTON_COL, + ReviewContext, + _render_constraint_violations_table, + _render_flag_correction_form, + _render_outlier_column_inspection, + outliers_report, +) +from datasure.checks.outliers.review import FlagSelection +from datasure.processing.correction_log import Action +from datasure.utils.correction_form import CorrectionFormState +from datasure.utils.dataframe_utils import ColumnByType +from tests.checks.outliers.conftest import _columns_side_effect + +MODULE = "datasure.checks.outliers.report_ui" + + +@pytest.fixture +def settings() -> OutlierSettings: + return OutlierSettings(survey_key="KEY", survey_id="hhid") + + +@pytest.fixture +def data() -> pl.DataFrame: + return pl.DataFrame( + { + "KEY": ["K1", "K2", "K3"], + "hhid": ["H1", "H2", "H3"], + "age": [150, 70, 30], + } + ) + + +@pytest.fixture +def violations() -> pl.DataFrame: + """`compute_constraint_violations` output for `data`.""" + return pl.DataFrame( + { + "KEY": ["K1", "K2", "K3"], + "column name": ["age", "age", "age"], + "column value": [150.0, 70.0, 30.0], + "hard_min": [0.0] * 3, + "soft_min": [None] * 3, + "soft_max": [65.0] * 3, + "hard_max": [100.0] * 3, + "violation reason": [ + "Value is above hard maximum 100.0", + "Value is above soft maximum 65.0", + "no violation", + ], + } + ) + + +def _acceptances(*rows: tuple[str, ...]) -> pl.DataFrame: + """Active acceptances with (KEY, column, reason[, severity]) rows.""" + return pl.DataFrame( + { + "KEY": [r[0] for r in rows], + "action": ["accept"] * len(rows), + "column": [r[1] for r in rows], + "reason": [r[2] for r in rows], + "severity": [r[3] if len(r) > 3 else None for r in rows], + }, + schema={ + "KEY": pl.String, + "action": pl.String, + "column": pl.String, + "reason": pl.String, + "severity": pl.String, + }, + ) + + +def _review( + acceptances_by_check: dict[str, pl.DataFrame] | None = None, + corrections: pl.DataFrame | None = None, +): + """A review context; `corrections` are active value corrections.""" + acceptances_by_check = acceptances_by_check or {} + processor = MagicMock() + processor.get_active_acceptances.side_effect = lambda alias, check_type, key_col: ( + acceptances_by_check.get(check_type, _acceptances()) + ) + processor.get_active_corrections.return_value = ( + _acceptances() if corrections is None else corrections + ) + return ReviewContext(processor=processor, alias="survey") + + +def _st_mock( + clicked: tuple[str, int] | None = None, + show_reviewed=False, + flagged_only=True, + reviewed_only=False, +): + """A Streamlit mock; `clicked` is (check type, row) of a Review click.""" + st_mock = MagicMock() + st_mock.columns.side_effect = _columns_side_effect + st_mock.multiselect.return_value = [] + toggle_values = { + "_flagged_only": flagged_only, + "_show_reviewed": show_reviewed, + "_reviewed_only": reviewed_only, + } + st_mock.toggle.side_effect = lambda label, *, key, **kwargs: next( + value for suffix, value in toggle_values.items() if key.endswith(suffix) + ) + st_mock.session_state = {} + # Widget state is in session_state before the widget renders. + for suffix, value in toggle_values.items(): + for check_type in ("constraints", "outliers"): + st_mock.session_state[f"{check_type}{suffix}"] = value + if clicked is not None: + check_type, row = clicked + st_mock.session_state[f"{check_type}_flag_review_click"] = { + "row": row, + "label": "Review", + } + return st_mock + + +def _shown_table(st_mock) -> pl.DataFrame: + """The flags shown, without the Review button column.""" + shown = st_mock.dataframe.call_args.args[0] + if isinstance(shown, Styler): + shown = pl.from_pandas(shown.data) + return shown.drop(REVIEW_BUTTON_COL, strict=False) + + +def _selection(**overrides) -> FlagSelection: + values = { + "key_value": "K2", + "column": "age", + "check_type": "constraints", + "flagged": True, + "reviewed": False, + "hard": False, + } + return FlagSelection(**(values | overrides)) + + +def _form_state(action=Action.ACCEPT, reason="verified", **overrides): + values = { + "key_value": "K2", + "action": action, + "column": "age", + "current_value": 70, + "reason": reason, + "check_type": "constraints" if action == Action.ACCEPT else None, + "survey_id_value": "H2", + } + return CorrectionFormState(**(values | overrides)) + + +class TestConstraintTableReviewButton: + def _render(self, data, violations, settings, st_mock, review): + with ( + patch(f"{MODULE}.st", st_mock), + patch(f"{MODULE}.load_check_settings", return_value={}), + patch(f"{MODULE}.save_check_settings"), + patch(f"{MODULE}._flag_correction_dialog") as dialog, + # styled_dataframe imports streamlit itself; forward to the mock. + patch( + f"{MODULE}.styled_dataframe", + side_effect=lambda styler, **kw: st_mock.dataframe(styler, **kw), + ), + ): + _render_constraint_violations_table( + data, violations, settings, "settings.json", review=review + ) + return dialog + + def test_first_column_is_a_review_button_on_every_flag( + self, data, violations, settings + ): + st_mock = _st_mock() + + self._render(data, violations, settings, st_mock, _review()) + + shown = st_mock.dataframe.call_args.args[0] + assert shown.columns[0] == REVIEW_BUTTON_COL + assert all("Review" in label for label in shown[REVIEW_BUTTON_COL]) + button_config = st_mock.dataframe.call_args.kwargs["column_config"][ + REVIEW_BUTTON_COL + ] + assert button_config is st_mock.column_config.ButtonColumn.return_value + button_kwargs = st_mock.column_config.ButtonColumn.call_args.kwargs + assert button_kwargs["key"] == "constraints_flag_review_click" + assert button_kwargs["pinned"] is True + + def test_button_column_does_not_clash_with_a_survey_column( + self, violations, settings + ): + """A survey field named like the button column can be shown too.""" + data = pl.DataFrame( + { + "KEY": ["K1", "K2", "K3"], + "hhid": ["H1", "H2", "H3"], + "_review": [1, 2, 3], + } + ) + st_mock = _st_mock(clicked=("constraints", 0)) + st_mock.multiselect.return_value = ["_review"] + + dialog = self._render(data, violations, settings, st_mock, _review()) + + shown = st_mock.dataframe.call_args.args[0] + (button_col,) = st_mock.dataframe.call_args.kwargs["column_config"] + assert button_col != REVIEW_BUTTON_COL + assert shown.columns[0] == button_col + assert shown["_review"].to_list() == [1, 2] + assert dialog.call_args.args[2].key_value == "K1" + + def test_a_survey_review_status_field_is_not_read_as_review_state( + self, violations, settings + ): + """A survey field named "review status" is shown, renamed, and ignored.""" + data = pl.DataFrame( + { + "KEY": ["K1", "K2", "K3"], + "hhid": ["H1", "H2", "H3"], + "review status": ["Reviewed"] * 3, + } + ) + st_mock = _st_mock(clicked=("constraints", 1)) + st_mock.multiselect.return_value = ["review status"] + + dialog = self._render(data, violations, settings, st_mock, _review()) + + shown = st_mock.dataframe.call_args.args[0] + assert not isinstance(shown, Styler) + assert "review status" not in shown.columns + assert shown["review status (survey)"].to_list() == ["Reviewed"] * 2 + selection = dialog.call_args.args[2] + assert selection.key_value == "K2" + assert not selection.reviewed + + def test_rows_are_not_selectable(self, data, violations, settings): + st_mock = _st_mock() + + self._render(data, violations, settings, st_mock, _review()) + + assert "on_select" not in st_mock.dataframe.call_args.kwargs + + def test_clicking_review_opens_the_dialog_prefilled_from_the_row( + self, data, violations, settings + ): + st_mock = _st_mock(clicked=("constraints", 0)) + review = _review() + + dialog = self._render(data, violations, settings, st_mock, review) + + dialog.assert_called_once() + _, _, selection, passed_review = dialog.call_args.args + assert selection == FlagSelection( + key_value="K1", + column="age", + check_type="constraints", + flagged=True, + reviewed=False, + hard=True, + ) + assert passed_review is review + + def test_no_click_opens_no_dialog(self, data, violations, settings): + dialog = self._render(data, violations, settings, _st_mock(), _review()) + + dialog.assert_not_called() + + def test_a_click_on_the_other_table_opens_no_dialog( + self, data, violations, settings + ): + st_mock = _st_mock(clicked=("outliers", 0)) + + dialog = self._render(data, violations, settings, st_mock, _review()) + + dialog.assert_not_called() + + def test_accepted_violations_are_hidden(self, data, violations, settings): + st_mock = _st_mock() + review = _review( + {"constraints": _acceptances(("K1", "age", "verified", "hard"))} + ) + + self._render(data, violations, settings, st_mock, review) + + assert _shown_table(st_mock)["KEY"].to_list() == ["K2"] + + def test_show_reviewed_shows_them_with_badge_and_reason( + self, data, violations, settings + ): + st_mock = _st_mock(show_reviewed=True) + review = _review( + {"constraints": _acceptances(("K1", "age", "verified", "hard"))} + ) + + self._render(data, violations, settings, st_mock, review) + + table = _shown_table(st_mock) + assert table.select("KEY", "review status", "review reason").rows() == [ + ("K1", "Reviewed", "verified"), + ("K2", None, None), + ] + + def test_show_reviewed_colours_reviewed_rows_green( + self, data, violations, settings + ): + st_mock = _st_mock(show_reviewed=True) + review = _review( + {"constraints": _acceptances(("K1", "age", "verified", "hard"))} + ) + + self._render(data, violations, settings, st_mock, review) + + shown = st_mock.dataframe.call_args.args[0] + assert isinstance(shown, Styler) + # Styler.ctx maps (row, column) to the CSS properties applied to it. + cell_styles = shown._compute().ctx + styled_rows = {row for (row, _), props in cell_styles.items() if props} + assert styled_rows == {0} + assert all( + prop == "background-color" + for props in cell_styles.values() + for prop, _ in props + ) + + def test_corrected_value_shows_highlighted_with_show_reviewed( + self, data, violations, settings + ): + """K3 was corrected into range: no longer flagged, but reviewed.""" + st_mock = _st_mock(show_reviewed=True, flagged_only=False) + review = _review(corrections=_acceptances(("K3", "age", "typo fixed"))) + + self._render(data, violations, settings, st_mock, review) + + table = _shown_table(st_mock) + assert table.filter(pl.col("KEY") == "K3").select( + "review status", "review reason" + ).rows() == [("Corrected", "typo fixed")] + cell_styles = st_mock.dataframe.call_args.args[0]._compute().ctx + styled_rows = {row for (row, _), props in cell_styles.items() if props} + assert styled_rows == {2} + review.processor.get_active_corrections.assert_called_with("survey", "KEY") + + def test_show_only_reviewed_lists_accepted_and_corrected_rows( + self, data, violations, settings + ): + st_mock = _st_mock(reviewed_only=True) + review = _review( + {"constraints": _acceptances(("K1", "age", "verified", "hard"))}, + corrections=_acceptances(("K3", "age", "typo fixed")), + ) + + self._render(data, violations, settings, st_mock, review) + + table = _shown_table(st_mock) + assert table.select("KEY", "review status").rows() == [ + ("K1", "Reviewed"), + ("K3", "Corrected"), + ] + assert isinstance(st_mock.dataframe.call_args.args[0], Styler) + + def test_show_only_reviewed_disables_the_other_toggles( + self, data, violations, settings + ): + st_mock = _st_mock(reviewed_only=True) + + self._render(data, violations, settings, st_mock, _review()) + + disabled = { + c.kwargs["key"]: c.kwargs.get("disabled", False) + for c in st_mock.toggle.call_args_list + } + assert disabled == { + "constraints_flagged_only": True, + "constraints_show_reviewed": True, + "constraints_reviewed_only": False, + } + + def test_without_review_there_is_no_show_only_reviewed_toggle( + self, data, violations, settings + ): + st_mock = _st_mock() + + self._render(data, violations, settings, st_mock, None) + + keys = [c.kwargs["key"] for c in st_mock.toggle.call_args_list] + assert keys == ["constraints_flagged_only"] + + def test_without_show_reviewed_the_table_is_not_styled( + self, data, violations, settings + ): + st_mock = _st_mock() + review = _review( + {"constraints": _acceptances(("K1", "age", "verified", "hard"))} + ) + + self._render(data, violations, settings, st_mock, review) + + assert not isinstance(st_mock.dataframe.call_args.args[0], Styler) + + def test_soft_acceptance_does_not_hide_a_hard_violation( + self, data, violations, settings + ): + """K1 was accepted as a soft violation; bounds now make it hard.""" + st_mock = _st_mock() + review = _review({"constraints": _acceptances(("K1", "age", "verified"))}) + + self._render(data, violations, settings, st_mock, review) + + assert _shown_table(st_mock)["KEY"].to_list() == ["K1", "K2"] + + def test_outlier_acceptance_does_not_hide_a_constraint_violation( + self, data, violations, settings + ): + st_mock = _st_mock() + review = _review({"outliers": _acceptances(("K1", "age", "verified"))}) + + self._render(data, violations, settings, st_mock, review) + + assert _shown_table(st_mock)["KEY"].to_list() == ["K1", "K2"] + + def test_flagged_only_toggle_is_on_by_default(self, data, violations, settings): + st_mock = _st_mock() + + self._render(data, violations, settings, st_mock, _review()) + + toggle_kwargs = { + c.kwargs["key"]: c.kwargs for c in st_mock.toggle.call_args_list + } + assert toggle_kwargs["constraints_flagged_only"]["value"] is True + + def test_turning_flagged_only_off_shows_every_checked_value( + self, data, violations, settings + ): + st_mock = _st_mock(flagged_only=False) + + self._render(data, violations, settings, st_mock, _review()) + + table = _shown_table(st_mock) + assert table.select("KEY", "violation type").rows() == [ + ("K1", "Hard Max"), + ("K2", "Soft Max"), + ("K3", None), + ] + + def test_unflagged_rows_offer_a_review_button_too(self, data, violations, settings): + st_mock = _st_mock(clicked=("constraints", 2), flagged_only=False) + + dialog = self._render(data, violations, settings, st_mock, _review()) + + selection = dialog.call_args.args[2] + assert selection.key_value == "K3" + assert selection.flagged is False + + def test_flagged_only_toggle_works_without_review(self, data, violations, settings): + st_mock = _st_mock(flagged_only=False) + + self._render(data, violations, settings, st_mock, None) + + assert _shown_table(st_mock)["KEY"].to_list() == ["K1", "K2", "K3"] + + def test_without_review_the_table_has_no_button(self, data, violations, settings): + st_mock = _st_mock() + + self._render(data, violations, settings, st_mock, None) + + assert REVIEW_BUTTON_COL not in st_mock.dataframe.call_args.args[0].columns + assert "column_config" not in st_mock.dataframe.call_args.kwargs + + +class TestOutlierTableReviewButton: + @pytest.fixture + def outliers(self) -> pl.DataFrame: + return pl.DataFrame( + { + "KEY": ["K1", "K2", "K3"], + "column name": ["age"] * 3, + "column value": [150.0, 70.0, 30.0], + "outlier reason": [ + "Value is above upper bound 120.00", + "no outlier", + "no outlier", + ], + } + ) + + def _render(self, data, outliers, settings, st_mock, review): + with ( + patch(f"{MODULE}.st", st_mock), + patch(f"{MODULE}.load_check_settings", return_value={}), + patch(f"{MODULE}.save_check_settings"), + patch(f"{MODULE}._create_descriptive_stats", return_value=pl.DataFrame()), + patch(f"{MODULE}._create_box_plot"), + patch(f"{MODULE}._flag_correction_dialog") as dialog, + # styled_dataframe imports streamlit itself; forward to the mock. + patch( + f"{MODULE}.styled_dataframe", + side_effect=lambda styler, **kw: st_mock.dataframe(styler, **kw), + ), + ): + st_mock.selectbox.return_value = "age" + _render_outlier_column_inspection( + data, outliers, settings, "settings.json", review=review + ) + return dialog + + def test_clicking_review_opens_the_dialog_for_outliers( + self, data, outliers, settings + ): + st_mock = _st_mock(clicked=("outliers", 0)) + + dialog = self._render(data, outliers, settings, st_mock, _review()) + + selection = dialog.call_args.args[2] + assert selection.key_value == "K1" + assert selection.column == "age" + assert selection.check_type == "outliers" + assert selection.hard is False + + def test_index_is_hidden_like_the_constraint_table(self, data, outliers, settings): + st_mock = _st_mock() + + self._render(data, outliers, settings, st_mock, _review()) + + assert st_mock.dataframe.call_args.kwargs["hide_index"] is True + + def test_show_reviewed_keeps_values_as_displayed(self, data, outliers, settings): + """Styling for "Show reviewed" must not turn 150 into 150.000000.""" + st_mock = _st_mock(show_reviewed=True, flagged_only=False) + review = _review({"outliers": _acceptances(("K1", "age", "verified"))}) + + self._render(data, outliers, settings, st_mock, review) + + styler = st_mock.dataframe.call_args.args[0] + body = styler._translate(False, False)["body"] + value_col = list(styler.data.columns).index("column value") + assert [row[value_col + 1]["display_value"] for row in body] == [ + "150.0", + "70.0", + "30.0", + ] + + def test_flagged_only_shows_only_outliers(self, data, outliers, settings): + st_mock = _st_mock() + + self._render(data, outliers, settings, st_mock, _review()) + + assert _shown_table(st_mock)["KEY"].to_list() == ["K1"] + + def test_turning_flagged_only_off_shows_every_checked_value( + self, data, outliers, settings + ): + st_mock = _st_mock(flagged_only=False) + + self._render(data, outliers, settings, st_mock, _review()) + + assert _shown_table(st_mock)["KEY"].to_list() == ["K1", "K2", "K3"] + + def test_accepted_outliers_are_hidden(self, data, outliers, settings): + st_mock = _st_mock() + review = _review({"outliers": _acceptances(("K1", "age", "verified"))}) + + self._render(data, outliers, settings, st_mock, review) + + assert "K1" not in _shown_table(st_mock)["KEY"].to_list() + + +class TestFlagCorrectionForm: + def _render(self, data, settings, selection, review, state, *, apply, confirm): + st_mock = _st_mock() + st_mock.button.return_value = apply + st_mock.checkbox.return_value = confirm + with ( + patch(f"{MODULE}.st", st_mock), + patch(f"{MODULE}.render_correction_inputs", return_value=state) as inputs, + patch( + f"{MODULE}.apply_correction_entries", return_value=True + ) as apply_entries, + ): + _render_flag_correction_form(data, settings, selection, review) + return st_mock, inputs, apply_entries + + def test_form_is_prefilled_from_the_selection_and_data(self, data, settings): + _, inputs, _ = self._render( + data, + settings, + _selection(), + _review(), + _form_state(), + apply=False, + confirm=False, + ) + + args, kwargs = inputs.call_args + assert args == (data, "KEY", "K2") + assert kwargs["column"] == "age" + assert kwargs["current_value"] == 70 + assert kwargs["survey_id_value"] == "H2" + assert kwargs["check_type"] == "constraints" + assert kwargs["actions"] == [ + Action.MODIFY_VALUE, + Action.REMOVE_VALUE, + Action.ACCEPT, + ] + + def test_apply_saves_with_the_check_as_source_and_reruns(self, data, settings): + review = _review() + + st_mock, _, apply_entries = self._render( + data, + settings, + _selection(), + review, + _form_state(action=Action.MODIFY_VALUE, new_value="60"), + apply=True, + confirm=False, + ) + + args, kwargs = apply_entries.call_args + assert args[:3] == (review.processor, "survey", "KEY") + (entry,) = args[3] + assert entry.action == Action.MODIFY_VALUE + assert entry.new_value == "60" + assert kwargs["source"] == "constraints" + st_mock.rerun.assert_called_once() + + def test_a_save_queues_a_toast_for_after_the_rerun(self, data, settings): + with patch(f"{MODULE}.queue_notice") as queue_notice: + self._render( + data, + settings, + _selection(), + _review(), + _form_state(), + apply=True, + confirm=False, + ) + + scope, level, message = queue_notice.call_args.args + assert scope == "outliers_corrections" + assert level == "toast" + assert "Correction Log" in message + + def test_hard_violation_accept_is_disabled_until_confirmed(self, data, settings): + st_mock, _, apply_entries = self._render( + data, + settings, + _selection(key_value="K1", hard=True), + _review(), + _form_state(key_value="K1", current_value=150), + apply=False, + confirm=False, + ) + + st_mock.checkbox.assert_called_once() + assert st_mock.button.call_args.kwargs["disabled"] is True + apply_entries.assert_not_called() + + def test_confirmed_hard_violation_accept_is_logged_as_hard(self, data, settings): + st_mock, _, apply_entries = self._render( + data, + settings, + _selection(key_value="K1", hard=True), + _review(), + _form_state(key_value="K1", current_value=150), + apply=True, + confirm=True, + ) + + assert st_mock.button.call_args.kwargs["disabled"] is False + (entry,) = apply_entries.call_args.args[3] + assert entry.action == Action.ACCEPT + assert entry.severity == "hard" + + def test_soft_violation_accept_needs_no_confirmation(self, data, settings): + st_mock, _, apply_entries = self._render( + data, + settings, + _selection(), + _review(), + _form_state(), + apply=True, + confirm=False, + ) + + st_mock.checkbox.assert_not_called() + (entry,) = apply_entries.call_args.args[3] + assert entry.severity is None + + def test_modifying_a_hard_violation_needs_no_confirmation(self, data, settings): + st_mock, _, _ = self._render( + data, + settings, + _selection(key_value="K1", hard=True), + _review(), + _form_state(action=Action.MODIFY_VALUE, new_value="90"), + apply=False, + confirm=False, + ) + + st_mock.checkbox.assert_not_called() + + def test_numeric_keys_are_saved_with_their_native_type(self, settings): + """The form shows KEY 7 as text, but validation compares native values.""" + numeric = pl.DataFrame({"KEY": [7, 8], "hhid": ["H7", "H8"], "age": [150, 30]}) + + _, inputs, apply_entries = self._render( + numeric, + settings, + _selection(key_value=7), + _review(), + _form_state(key_value="7", action=Action.MODIFY_VALUE, new_value="90"), + apply=True, + confirm=False, + ) + + assert inputs.call_args.args[2] == "7" + assert inputs.call_args.kwargs["current_value"] == 150 + (entry,) = apply_entries.call_args.args[3] + assert entry.key_value == 7 + + def test_widget_keys_are_unique_per_cell(self, data, settings): + """KEY "survey_1"/column "age" and KEY "survey"/column "1_age" differ.""" + namespaces = [] + for key, column in (("survey_1", "age"), ("survey", "1_age")): + _, inputs, _ = self._render( + data, + settings, + _selection(key_value=key, column=column), + _review(), + _form_state(key_value=key, column=column), + apply=False, + confirm=False, + ) + namespaces.append(inputs.call_args.kwargs["key_namespace"]) + + assert namespaces[0] != namespaces[1] + + def test_duplicate_key_with_different_values_cannot_be_reviewed(self, settings): + """Reviewing age 150 must not prefill or overwrite the other K1's 30.""" + duplicated = pl.DataFrame( + {"KEY": ["K1", "K1"], "hhid": ["H1", "H1"], "age": [30, 150]} + ) + + st_mock, inputs, apply_entries = self._render( + duplicated, + settings, + _selection(key_value="K1", hard=True), + _review(), + _form_state(key_value="K1", action=Action.MODIFY_VALUE, new_value="90"), + apply=True, + confirm=True, + ) + + assert "more than one row" in st_mock.warning.call_args.args[0] + inputs.assert_not_called() + apply_entries.assert_not_called() + st_mock.button.assert_not_called() + + def test_duplicate_key_with_the_same_value_can_be_reviewed(self, settings): + duplicated = pl.DataFrame( + {"KEY": ["K1", "K1"], "hhid": ["H1", "H1"], "age": [150, 150]} + ) + + _, inputs, _ = self._render( + duplicated, + settings, + _selection(key_value="K1"), + _review(), + _form_state(key_value="K1"), + apply=False, + confirm=False, + ) + + assert inputs.call_args.kwargs["current_value"] == 150 + + def test_failed_save_does_not_rerun(self, data, settings): + st_mock = _st_mock() + st_mock.button.return_value = True + with ( + patch(f"{MODULE}.st", st_mock), + patch(f"{MODULE}.render_correction_inputs", return_value=_form_state()), + patch(f"{MODULE}.apply_correction_entries", return_value=False), + ): + _render_flag_correction_form(data, settings, _selection(), _review()) + + st_mock.rerun.assert_not_called() + + +class TestMetricsExcludeAcceptedFlags: + @pytest.fixture + def outliers(self) -> pl.DataFrame: + return pl.DataFrame( + { + "KEY": ["K1", "K2", "K3"], + "column name": ["age"] * 3, + "column value": [150.0, 70.0, 30.0], + "outlier reason": [ + "Value is above upper bound 120.00", + "Value is above upper bound 60.00", + "no outlier", + ], + } + ) + + def _run_report( + self, data, violations, outliers, acceptances_by_check, survey_key="KEY" + ): + config = {"survey_key": survey_key, "survey_id": "hhid"} + columns = ColumnByType( + all_columns=data.columns, + categorical_columns=[], + datetime_columns=[], + numeric_columns=["age"], + string_columns=[], + integer_columns=["age"], + ) + processor = _review(acceptances_by_check).processor + self.st_mock = _st_mock() + with ( + patch(f"{MODULE}.st", self.st_mock), + patch( + f"{MODULE}.outliers_report_settings", + return_value=OutlierSettings(**config), + ), + patch(f"{MODULE}._render_outlier_column_actions"), + patch( + f"{MODULE}.duckdb_get_table", + return_value=pl.DataFrame({"column_name": [["age"]]}), + ), + patch(f"{MODULE}._update_unlocked_cols", side_effect=lambda df, _: df), + patch(f"{MODULE}.duckdb_save_table"), + patch(f"{MODULE}.compute_constraint_violations", return_value=violations), + patch(f"{MODULE}.compute_outlier_output", return_value=outliers), + patch(f"{MODULE}.CorrectionProcessor", return_value=processor), + patch(f"{MODULE}._render_constraint_metrics") as constraint_metrics, + patch(f"{MODULE}._render_constraint_violations_table") as table, + patch(f"{MODULE}._render_outlier_metrics") as outlier_metrics, + patch(f"{MODULE}._render_outlier_column_inspection") as inspection, + ): + outliers_report( + "proj1", + "page1", + data, + "settings.json", + config, + columns, + alias="survey", + ) + self.processor = processor + return constraint_metrics, table, outlier_metrics, inspection + + def test_constraint_metrics_do_not_count_accepted_violations( + self, data, violations, outliers + ): + metrics, table, _, _ = self._run_report( + data, + violations, + outliers, + {"constraints": _acceptances(("K1", "age", "verified", "hard"))}, + ) + + counted = metrics.call_args.args[0] + assert counted.filter(pl.col("violation reason") != "no violation")[ + "KEY" + ].to_list() == ["K2"] + assert table.call_args.kwargs["review"].alias == "survey" + + def test_outlier_metrics_do_not_count_accepted_outliers( + self, data, violations, outliers + ): + _, _, metrics, inspection = self._run_report( + data, + violations, + outliers, + {"outliers": _acceptances(("K2", "age", "verified"))}, + ) + + counted = metrics.call_args.args[0] + assert counted.filter(pl.col("outlier reason") != "no outlier")[ + "KEY" + ].to_list() == ["K1"] + # Rows are kept, so the column still counts as checked. + assert counted.height == outliers.height + assert inspection.call_args.kwargs["review"].alias == "survey" + + def test_corrections_are_looked_up_once_per_report_run( + self, data, violations, outliers + ): + self._run_report(data, violations, outliers, {}) + + assert self.processor.get_active_corrections.call_count == 1 + + def test_a_key_named_like_a_review_column_turns_review_off( + self, data, violations, outliers + ): + """The report still renders, without overwriting the KEY column.""" + key = "review status" + + _, table, _, inspection = self._run_report( + data.rename({"KEY": key}), + violations.rename({"KEY": key}), + outliers.rename({"KEY": key}), + {"constraints": _acceptances(("K1", "age", "verified", "hard"))}, + survey_key=key, + ) + + assert key in self.st_mock.warning.call_args.args[0] + assert table.call_args.kwargs["review"] is None + assert inspection.call_args.kwargs["review"] is None + assert table.call_args.args[1][key].to_list() == ["K1", "K2", "K3"] + self.processor.get_active_acceptances.assert_not_called() diff --git a/tests/checks/outliers/test_review.py b/tests/checks/outliers/test_review.py new file mode 100644 index 00000000..e1a48a1f --- /dev/null +++ b/tests/checks/outliers/test_review.py @@ -0,0 +1,765 @@ +"""Tests for datasure.checks.outliers.review.""" + +import pandas as pd +import polars as pl +import pytest + +from datasure.checks.outliers.review import ( + CONSTRAINTS, + CORRECTED_BADGE, + OUTLIERS, + REVIEW_COLUMNS, + REVIEW_REASON_COL, + REVIEW_STATUS_COL, + REVIEWED_BADGE, + SURVEY_COL_SUFFIX, + VIOLATION_TYPE_COL, + FlagSelection, + TableFilters, + allowed_actions, + clear_reviewed_flags, + filter_table, + flagged_only, + highlight_reviewed_row, + join_survey_columns, + key_has_conflicting_values, + mark_reviewed, + needs_hard_confirmation, + select_flag, + visible_flags, +) +from datasure.processing.correction_log import Action, empty_correction_log + + +def _acceptances(rows: list[dict]) -> pl.DataFrame: + """Build accept rows shaped like `get_active_acceptances` output.""" + base = empty_correction_log() + if not rows: + return base + return pl.DataFrame( + [ + { + "KEY": r["KEY"], + "action": "accept", + "column": r["column"], + "current_value": r.get("current_value"), + "reason": r.get("reason", "checked"), + "check_type": r.get("check_type", "outliers"), + "severity": r.get("severity"), + } + for r in rows + ] + ).select( + pl.col(c).cast(pl.String) + for c in [ + "KEY", + "action", + "column", + "current_value", + "reason", + "check_type", + "severity", + ] + ) + + +@pytest.fixture +def outlier_flags() -> pl.DataFrame: + return pl.DataFrame( + { + "survey_key": ["K1", "K2", "K3", "K1"], + "column name": ["age", "age", "age", "income"], + "column value": [99.0, 30.0, 120.0, 5000.0], + "outlier reason": [ + "Value is above upper bound 80.00", + "no outlier", + "Value is above upper bound 80.00", + "Value is above upper bound 900.00", + ], + } + ) + + +class TestMarkReviewed: + def test_flags_matching_an_acceptance_are_reviewed_with_its_reason( + self, outlier_flags + ): + acceptances = _acceptances( + [{"KEY": "K1", "column": "age", "reason": "verified by phone"}] + ) + + result = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + assert result[REVIEW_STATUS_COL].to_list() == [ + REVIEWED_BADGE, + None, + None, + None, + ] + assert result[REVIEW_REASON_COL].to_list() == [ + "verified by phone", + None, + None, + None, + ] + + def test_acceptance_on_another_column_of_the_same_key_does_not_match( + self, outlier_flags + ): + acceptances = _acceptances([{"KEY": "K1", "column": "income"}]) + + result = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + assert result[REVIEW_STATUS_COL].to_list() == [ + None, + None, + None, + REVIEWED_BADGE, + ] + + def test_unflagged_rows_are_never_reviewed(self, outlier_flags): + acceptances = _acceptances([{"KEY": "K2", "column": "age"}]) + + result = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + assert result[REVIEW_STATUS_COL].null_count() == result.height + + def test_keeps_row_order_and_columns(self, outlier_flags): + acceptances = _acceptances([{"KEY": "K3", "column": "age"}]) + + result = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + assert result.columns == [ + *outlier_flags.columns, + REVIEW_STATUS_COL, + REVIEW_REASON_COL, + ] + assert result.drop(REVIEW_STATUS_COL, REVIEW_REASON_COL).equals(outlier_flags) + + def test_matches_non_string_keys_as_text(self): + flags = pl.DataFrame( + { + "survey_key": [1, 2], + "column name": ["age", "age"], + "violation reason": ["Value is above hard maximum 100", "no violation"], + } + ) + acceptances = _acceptances( + [ + { + "KEY": "1", + "column": "age", + "check_type": "constraints", + "severity": "hard", + } + ] + ) + + result = mark_reviewed(flags, acceptances, "survey_key", CONSTRAINTS) + + assert result[REVIEW_STATUS_COL].to_list() == [REVIEWED_BADGE, None] + + def test_repeated_acceptances_use_the_latest_reason(self, outlier_flags): + acceptances = _acceptances( + [ + {"KEY": "K1", "column": "age", "reason": "first"}, + {"KEY": "K1", "column": "age", "reason": "second"}, + ] + ) + + result = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + assert result.height == outlier_flags.height + assert result[REVIEW_REASON_COL][0] == "second" + + def test_no_acceptances_marks_nothing(self, outlier_flags): + result = mark_reviewed( + outlier_flags, empty_correction_log(), "survey_key", OUTLIERS + ) + + assert result[REVIEW_STATUS_COL].null_count() == result.height + + def test_empty_flags_are_returned_unchanged(self): + result = mark_reviewed(pl.DataFrame(), _acceptances([]), "survey_key", OUTLIERS) + + assert result.is_empty() + + @pytest.mark.parametrize("key_name", ["_key", "_review_key", "_accept_reason"]) + def test_a_key_named_like_a_helper_column_is_kept(self, outlier_flags, key_name): + flags = outlier_flags.rename({"survey_key": key_name}) + acceptances = _acceptances([{"KEY": "K1", "column": "age"}]) + + result = mark_reviewed(flags, acceptances, key_name, OUTLIERS) + + assert result.columns == [*flags.columns, REVIEW_STATUS_COL, REVIEW_REASON_COL] + assert result[key_name].to_list() == flags[key_name].to_list() + assert result[REVIEW_STATUS_COL][0] == REVIEWED_BADGE + + @pytest.mark.parametrize("key_name", REVIEW_COLUMNS) + def test_a_key_named_like_a_review_column_is_rejected( + self, outlier_flags, key_name + ): + flags = outlier_flags.rename({"survey_key": key_name}) + + with pytest.raises(ValueError, match="review column"): + mark_reviewed(flags, _acceptances([]), key_name, OUTLIERS) + + +class TestClearReviewedFlags: + def test_reviewed_flags_no_longer_count_as_flags(self, outlier_flags): + marked = mark_reviewed( + outlier_flags, + _acceptances([{"KEY": "K1", "column": "age"}]), + "survey_key", + OUTLIERS, + ) + + result = clear_reviewed_flags(marked, OUTLIERS) + + assert result["outlier reason"].to_list() == [ + "no outlier", + "no outlier", + "Value is above upper bound 80.00", + "Value is above upper bound 900.00", + ] + + def test_keeps_rows_so_checked_columns_are_still_counted(self, outlier_flags): + acceptances = _acceptances( + [{"KEY": "K1", "column": "income"}, {"KEY": "K1", "column": "age"}] + ) + marked = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + result = clear_reviewed_flags(marked, OUTLIERS) + + assert result.height == outlier_flags.height + assert set(result["column name"]) == {"age", "income"} + + def test_data_without_review_columns_is_unchanged(self, outlier_flags): + assert clear_reviewed_flags(outlier_flags, OUTLIERS).equals(outlier_flags) + + +class TestHighlightReviewedRow: + def test_reviewed_rows_are_green_in_every_cell(self): + row = pd.Series({"KEY": "K1", REVIEW_STATUS_COL: REVIEWED_BADGE}) + + styles = highlight_reviewed_row(row) + + assert len(styles) == len(row) + assert all("background-color" in style for style in styles) + assert all("25, 135, 84" in style for style in styles) + + @pytest.mark.parametrize("status", [None, float("nan"), pd.NA]) + def test_other_rows_are_plain(self, status): + row = pd.Series({"KEY": "K2", REVIEW_STATUS_COL: status}) + + assert highlight_reviewed_row(row) == ["", ""] + + +class TestFlaggedOnly: + def test_keeps_only_flagged_rows(self, outlier_flags): + result = flagged_only(outlier_flags, OUTLIERS) + + assert result["survey_key"].to_list() == ["K1", "K3", "K1"] + assert "no outlier" not in result["outlier reason"].to_list() + + def test_uses_the_check_sentinel(self): + flags = pl.DataFrame( + { + "survey_key": ["K1", "K2"], + "column name": ["age", "age"], + "violation reason": ["Value is above hard maximum 100", "no violation"], + } + ) + + assert flagged_only(flags, CONSTRAINTS)["survey_key"].to_list() == ["K1"] + + def test_data_without_the_reason_column_is_unchanged(self): + flags = pl.DataFrame({"survey_key": ["K1"]}) + + assert flagged_only(flags, OUTLIERS).equals(flags) + + +class TestVisibleFlags: + @pytest.fixture + def marked(self, outlier_flags): + return mark_reviewed( + outlier_flags, + _acceptances([{"KEY": "K1", "column": "age", "reason": "ok"}]), + "survey_key", + OUTLIERS, + ) + + def test_hides_reviewed_flags_and_review_columns(self, marked): + result = visible_flags(marked, show_reviewed=False) + + assert result.height == 3 + assert REVIEW_STATUS_COL not in result.columns + assert REVIEW_REASON_COL not in result.columns + + def test_show_reviewed_keeps_them_with_badge_and_reason(self, marked): + result = visible_flags(marked, show_reviewed=True) + + assert result.height == 4 + assert result.row(0, named=True)[REVIEW_STATUS_COL] == REVIEWED_BADGE + assert result.row(0, named=True)[REVIEW_REASON_COL] == "ok" + + def test_data_without_review_columns_is_unchanged(self, outlier_flags): + assert visible_flags(outlier_flags, show_reviewed=False).equals(outlier_flags) + + +@pytest.fixture +def constraint_table() -> pl.DataFrame: + """A constraint table as displayed: flagged rows plus the violation type.""" + return pl.DataFrame( + { + "survey_key": ["K1", "K2"], + "survey_id": ["S1", "S2"], + "column name": ["age", "age"], + "column value": [150.0, 70.0], + "violation reason": [ + "Value is above hard maximum 100", + "Value is above soft maximum 65", + ], + "violation type": ["Hard Max", "Soft Max"], + } + ) + + +class TestJoinSurveyColumns: + def test_flag_column_name_wins_over_a_survey_field_of_that_name( + self, outlier_flags + ): + survey = pl.DataFrame( + {"survey_key": ["K1", "K2", "K3"], "column name": ["income"] * 3} + ) + + table = join_survey_columns(survey, outlier_flags, "survey_key", OUTLIERS) + selection = select_flag(table, [0], "survey_key", OUTLIERS) + + assert selection.column == "age" + assert table[f"column name{SURVEY_COL_SUFFIX}"].to_list() == ["income"] * 4 + + def test_reserved_names_are_renamed_on_the_survey_side(self, outlier_flags): + survey = pl.DataFrame( + { + "survey_key": ["K1", "K2", "K3"], + VIOLATION_TYPE_COL: ["a", "b", "c"], + f"{VIOLATION_TYPE_COL}{SURVEY_COL_SUFFIX}": ["x", "y", "z"], + } + ) + + table = join_survey_columns( + survey, + outlier_flags, + "survey_key", + OUTLIERS, + reserved=[VIOLATION_TYPE_COL], + ) + + assert VIOLATION_TYPE_COL not in table.columns + assert f"{VIOLATION_TYPE_COL}{SURVEY_COL_SUFFIX * 2}" in table.columns + assert f"{VIOLATION_TYPE_COL}{SURVEY_COL_SUFFIX}" in table.columns + + def test_survey_review_fields_are_renamed_while_review_columns_are_hidden( + self, outlier_flags + ): + """With "Show reviewed" off, survey text is not read as review state.""" + survey = pl.DataFrame( + { + "survey_key": ["K1", "K2", "K3"], + REVIEW_STATUS_COL: [REVIEWED_BADGE] * 3, + REVIEW_REASON_COL: ["survey text"] * 3, + } + ) + + table = join_survey_columns(survey, outlier_flags, "survey_key", OUTLIERS) + selection = select_flag(table, [0], "survey_key", OUTLIERS) + + assert REVIEW_STATUS_COL not in table.columns + assert REVIEW_REASON_COL not in table.columns + assert table[f"{REVIEW_STATUS_COL}{SURVEY_COL_SUFFIX}"][0] == REVIEWED_BADGE + assert not selection.reviewed + + def test_generated_review_columns_win_over_survey_fields(self, outlier_flags): + """With "Show reviewed" on, badges and reasons come from the log.""" + marked = mark_reviewed( + outlier_flags, + _acceptances([{"KEY": "K1", "column": "age", "reason": "verified"}]), + "survey_key", + OUTLIERS, + ) + survey = pl.DataFrame( + { + "survey_key": ["K1", "K2", "K3"], + REVIEW_STATUS_COL: ["survey status"] * 3, + REVIEW_REASON_COL: ["survey reason"] * 3, + } + ) + + table = join_survey_columns(survey, marked, "survey_key", OUTLIERS) + k1_age = table.filter( + (pl.col("survey_key") == "K1") & (pl.col("column name") == "age") + ) + + assert k1_age[REVIEW_STATUS_COL].to_list() == [REVIEWED_BADGE] + assert k1_age[REVIEW_REASON_COL].to_list() == ["verified"] + assert k1_age[f"{REVIEW_REASON_COL}{SURVEY_COL_SUFFIX}"].to_list() == [ + "survey reason" + ] + + def test_row_order_does_not_depend_on_input_order(self, outlier_flags): + survey = pl.DataFrame( + {"survey_key": ["K1", "K2", "K3"], "enumerator": ["E1", "E2", "E3"]} + ) + + table = join_survey_columns(survey, outlier_flags, "survey_key", OUTLIERS) + reordered = join_survey_columns( + survey.reverse(), outlier_flags.reverse(), "survey_key", OUTLIERS + ) + + assert table.equals(reordered) + # A click on row 1 of the first render resolves to the same flag after + # a rerun whose join returned rows in another order. + assert select_flag(table, [1], "survey_key", OUTLIERS) == select_flag( + reordered, [1], "survey_key", OUTLIERS + ) + + def test_duplicate_key_and_column_are_ordered_by_reason(self): + flags = pl.DataFrame( + { + "survey_key": ["K1", "K1"], + "column name": ["age", "age"], + "violation reason": [ + "Value is above soft maximum 65", + "Value is above hard maximum 100", + ], + } + ) + survey = pl.DataFrame({"survey_key": ["K1"]}) + + table = join_survey_columns(survey, flags, "survey_key", CONSTRAINTS) + + assert table["violation reason"].to_list() == [ + "Value is above hard maximum 100", + "Value is above soft maximum 65", + ] + + +class TestSelectFlag: + def test_prefills_key_and_column_from_the_selected_row(self, constraint_table): + selection = select_flag(constraint_table, [1], "survey_key", CONSTRAINTS) + + assert selection == FlagSelection( + key_value="K2", + column="age", + check_type="constraints", + flagged=True, + reviewed=False, + hard=False, + ) + + def test_hard_violations_are_marked_hard(self, constraint_table): + selection = select_flag(constraint_table, [0], "survey_key", CONSTRAINTS) + + assert selection.hard is True + + def test_outliers_are_never_hard(self, outlier_flags): + selection = select_flag(outlier_flags, [0], "survey_key", OUTLIERS) + + assert selection.check_type == "outliers" + assert selection.flagged is True + assert selection.hard is False + + def test_unflagged_rows_are_reported_as_unflagged(self, outlier_flags): + selection = select_flag(outlier_flags, [1], "survey_key", OUTLIERS) + + assert selection.flagged is False + + def test_reviewed_rows_are_reported_as_reviewed(self, outlier_flags): + table = visible_flags( + mark_reviewed( + outlier_flags, + _acceptances([{"KEY": "K1", "column": "age"}]), + "survey_key", + OUTLIERS, + ), + show_reviewed=True, + ) + + selection = select_flag(table, [0], "survey_key", OUTLIERS) + + assert selection.reviewed is True + + def test_keeps_the_key_value_as_stored_in_the_data(self): + table = pl.DataFrame( + { + "survey_key": [7], + "column name": ["age"], + "outlier reason": ["Value is above upper bound 80.00"], + } + ) + + assert select_flag(table, [0], "survey_key", OUTLIERS).key_value == 7 + + @pytest.mark.parametrize("rows", [[], [5], [-1]]) + def test_no_or_stale_selection_returns_none(self, constraint_table, rows): + assert select_flag(constraint_table, rows, "survey_key", CONSTRAINTS) is None + + +class TestKeyHasConflictingValues: + @pytest.mark.parametrize( + ("ages", "expected"), + [ + ([30, 150, 40], True), + ([150, 150, 40], False), + ([None, 150, 40], True), + ], + ids=["different-values", "same-value", "missing-and-value"], + ) + def test_duplicate_keys(self, ages, expected): + data = pl.DataFrame({"KEY": ["K1", "K1", "K2"], "age": ages}) + + assert key_has_conflicting_values(data, "KEY", "K1", "age") is expected + + def test_unique_key(self): + data = pl.DataFrame({"KEY": ["K1", "K2"], "age": [30, 150]}) + + assert not key_has_conflicting_values(data, "KEY", "K1", "age") + + def test_numeric_key_matches_its_native_value(self): + data = pl.DataFrame({"KEY": [7, 7], "age": [30, 150]}) + + assert key_has_conflicting_values(data, "KEY", 7, "age") + + def test_missing_column_is_not_a_conflict(self): + data = pl.DataFrame({"KEY": ["K1", "K1"], "age": [30, 150]}) + + assert not key_has_conflicting_values(data, "KEY", "K1", "income") + + +class TestAllowedActions: + def _selection(self, **overrides) -> FlagSelection: + values = { + "key_value": "K1", + "column": "age", + "check_type": "outliers", + "flagged": True, + "reviewed": False, + "hard": False, + } + return FlagSelection(**(values | overrides)) + + def test_flagged_value_can_be_modified_removed_or_accepted(self): + assert allowed_actions(self._selection()) == [ + Action.MODIFY_VALUE, + Action.REMOVE_VALUE, + Action.ACCEPT, + ] + + def test_unflagged_value_cannot_be_accepted(self): + assert Action.ACCEPT not in allowed_actions(self._selection(flagged=False)) + + def test_reviewed_value_cannot_be_accepted_again(self): + assert Action.ACCEPT not in allowed_actions(self._selection(reviewed=True)) + + def test_rows_are_never_removed_from_a_check_page(self): + assert Action.REMOVE_ROW not in allowed_actions(self._selection()) + + def test_only_accepting_a_hard_violation_needs_confirmation(self): + hard = self._selection(check_type="constraints", hard=True) + soft = self._selection(check_type="constraints", hard=False) + + assert needs_hard_confirmation(hard, Action.ACCEPT) is True + assert needs_hard_confirmation(hard, Action.MODIFY_VALUE) is False + assert needs_hard_confirmation(soft, Action.ACCEPT) is False + + +def _corrections(rows: list[dict]) -> pl.DataFrame: + """Active value corrections shaped like `get_active_corrections` output.""" + if not rows: + return empty_correction_log() + return pl.DataFrame( + { + "KEY": [r["KEY"] for r in rows], + "action": [r.get("action", "modify value") for r in rows], + "column": [r["column"] for r in rows], + "reason": [r.get("reason", "typo") for r in rows], + } + ) + + +class TestCorrectedValues: + """Cells whose current value comes from a correction are marked Corrected.""" + + def test_corrected_rows_get_the_corrected_badge_and_reason(self, outlier_flags): + # K2/age is no longer flagged after its correction. + corrections = _corrections([{"KEY": "K2", "column": "age", "reason": "typo"}]) + + result = mark_reviewed( + outlier_flags, _acceptances([]), "survey_key", OUTLIERS, corrections + ) + + assert result.row(1, named=True)[REVIEW_STATUS_COL] == CORRECTED_BADGE + assert result.row(1, named=True)[REVIEW_REASON_COL] == "typo" + + def test_an_acceptance_takes_precedence_over_a_correction(self, outlier_flags): + result = mark_reviewed( + outlier_flags, + _acceptances([{"KEY": "K1", "column": "age", "reason": "verified"}]), + "survey_key", + OUTLIERS, + _corrections([{"KEY": "K1", "column": "age", "reason": "typo"}]), + ) + + assert result.row(0, named=True)[REVIEW_STATUS_COL] == REVIEWED_BADGE + assert result.row(0, named=True)[REVIEW_REASON_COL] == "verified" + + def test_corrected_flags_stay_visible_and_counted(self, outlier_flags): + # K3/age was corrected but is still flagged. + marked = mark_reviewed( + outlier_flags, + _acceptances([]), + "survey_key", + OUTLIERS, + _corrections([{"KEY": "K3", "column": "age"}]), + ) + + visible = visible_flags(marked, show_reviewed=False) + counted = clear_reviewed_flags(marked, OUTLIERS) + + assert "K3" in visible["survey_key"].to_list() + assert counted["outlier reason"][2] == "Value is above upper bound 80.00" + + def test_corrected_rows_are_highlighted(self): + row = pd.Series({"KEY": "K1", REVIEW_STATUS_COL: CORRECTED_BADGE}) + + assert all(highlight_reviewed_row(row)) + + def test_a_corrected_flag_can_still_be_accepted(self, outlier_flags): + marked = mark_reviewed( + outlier_flags, + _acceptances([]), + "survey_key", + OUTLIERS, + _corrections([{"KEY": "K3", "column": "age"}]), + ) + + selection = select_flag(marked, [2], "survey_key", OUTLIERS) + + assert selection.reviewed is False + assert Action.ACCEPT in allowed_actions(selection) + + +class TestFilterTable: + """The table toggles applied together.""" + + @pytest.fixture + def marked(self, outlier_flags): + # K1/age accepted; K2/age corrected into range (unflagged). + return mark_reviewed( + outlier_flags, + _acceptances([{"KEY": "K1", "column": "age", "reason": "ok"}]), + "survey_key", + OUTLIERS, + _corrections([{"KEY": "K2", "column": "age", "reason": "typo"}]), + ) + + @staticmethod + def _cells(table): + return list(zip(table["survey_key"], table["column name"], strict=True)) + + def test_defaults_show_unreviewed_flags_only(self, marked): + result = filter_table(marked, TableFilters(), OUTLIERS) + + assert self._cells(result) == [("K3", "age"), ("K1", "income")] + assert REVIEW_STATUS_COL not in result.columns + + def test_show_reviewed_adds_accepted_flags(self, marked): + result = filter_table(marked, TableFilters(show_reviewed=True), OUTLIERS) + + assert self._cells(result) == [("K1", "age"), ("K3", "age"), ("K1", "income")] + + def test_all_values_with_reviewed(self, marked): + filters = TableFilters(flagged_only=False, show_reviewed=True) + + assert filter_table(marked, filters, OUTLIERS).height == marked.height + + def test_reviewed_only_shows_accepted_and_corrected_rows(self, marked): + result = filter_table(marked, TableFilters(reviewed_only=True), OUTLIERS) + + assert self._cells(result) == [("K1", "age"), ("K2", "age")] + assert result[REVIEW_STATUS_COL].to_list() == [REVIEWED_BADGE, CORRECTED_BADGE] + + def test_reviewed_only_overrides_the_other_toggles(self, marked): + filters = TableFilters( + flagged_only=True, show_reviewed=False, reviewed_only=True + ) + + result = filter_table(marked, filters, OUTLIERS) + + assert ("K2", "age") in self._cells(result) + + def test_reviewed_only_without_review_columns_shows_nothing(self, outlier_flags): + result = filter_table(outlier_flags, TableFilters(reviewed_only=True), OUTLIERS) + + assert result.is_empty() + assert result.columns == outlier_flags.columns + + +class TestHardViolationsNeedHardAcceptances: + """A soft acceptance must not silence a value that is now a hard violation.""" + + @pytest.fixture + def violations(self) -> pl.DataFrame: + # Bounds were tightened: age 70 is now above the hard maximum. + return pl.DataFrame( + { + "survey_key": ["K1", "K2"], + "column name": ["age", "age"], + "violation reason": [ + "Value is above hard maximum 60.0", + "Value is above soft maximum 50.0", + ], + } + ) + + def _mark(self, violations, severity): + acceptances = _acceptances( + [ + {"KEY": key, "column": "age", "check_type": "constraints", **severity} + for key in ("K1", "K2") + ] + ) + return mark_reviewed(violations, acceptances, "survey_key", CONSTRAINTS) + + def test_soft_acceptance_leaves_a_hard_violation_pending(self, violations): + marked = self._mark(violations, {}) + + assert marked[REVIEW_STATUS_COL].to_list() == [None, REVIEWED_BADGE] + + def test_hard_acceptance_covers_a_hard_violation(self, violations): + marked = self._mark(violations, {"severity": "hard"}) + + assert marked[REVIEW_STATUS_COL].to_list() == [REVIEWED_BADGE, REVIEWED_BADGE] + + def test_pending_hard_violation_is_counted_and_offers_confirmed_accept( + self, violations + ): + marked = self._mark(violations, {}) + table = marked.with_columns( + pl.Series("violation type", ["Hard Max", "Soft Max"]) + ) + + counted = clear_reviewed_flags(marked, CONSTRAINTS) + selection = select_flag(table, [0], "survey_key", CONSTRAINTS) + + assert counted["violation reason"][0] == "Value is above hard maximum 60.0" + assert Action.ACCEPT in allowed_actions(selection) + assert needs_hard_confirmation(selection, Action.ACCEPT) + + def test_acceptances_without_a_severity_column_still_work(self, outlier_flags): + acceptances = _acceptances([{"KEY": "K1", "column": "age"}]).drop("severity") + + marked = mark_reviewed(outlier_flags, acceptances, "survey_key", OUTLIERS) + + assert marked[REVIEW_STATUS_COL][0] == REVIEWED_BADGE diff --git a/tests/processing/test_corrections.py b/tests/processing/test_corrections.py index fba74060..285b480a 100644 --- a/tests/processing/test_corrections.py +++ b/tests/processing/test_corrections.py @@ -1169,6 +1169,7 @@ def test_removing_the_only_entry_leaves_an_empty_log_with_full_schema( "status_reason", "source", "check_type", + "severity", ] @@ -1829,3 +1830,283 @@ def test_reports_corrections_that_no_longer_apply(self, store, sample_data): assert len(failures) == 1 assert processor.get_correction_log("survey")["status"].to_list() == ["Failed"] + + +class TestAcceptanceSeverity: + """Hard constraint acceptances record their severity in the log.""" + + def _hard_accept(self, **overrides): + values = { + "key_value": "key1", + "action": "accept", + "check_type": "constraints", + "column": "age", + "current_value": 25, + "reason": "verified with respondent", + "severity": "hard", + } + return CorrectionEntry(**(values | overrides)) + + def test_apply_corrections_logs_the_severity(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + + processor.apply_corrections( + alias="survey", + key_col="survey_key", + entries=[self._hard_accept()], + source="constraints", + ) + + log = processor.get_correction_log("survey") + assert log.select("action", "source", "severity").rows() == [ + ("accept", "constraints", "hard") + ] + + def test_entries_without_severity_log_null(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + + processor.apply_corrections( + alias="survey", + key_col="survey_key", + entries=[self._hard_accept(severity=None)], + source="constraints", + ) + + assert processor.get_correction_log("survey")["severity"].to_list() == [None] + + def test_severity_is_only_recorded_on_acceptances(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + + with pytest.raises(ValueError, match="severity"): + processor.apply_corrections( + alias="survey", + key_col="survey_key", + entries=[ + self._hard_accept( + action="modify value", new_value=26, check_type=None + ) + ], + source="constraints", + ) + + assert processor.get_correction_log("survey").is_empty() + + @pytest.mark.parametrize( + "overrides", + [{"check_type": "outliers"}, {"severity": "soft"}], + ids=["hard-outlier-acceptance", "unknown-severity"], + ) + def test_invalid_acceptance_severity_is_rejected( + self, store, sample_data, overrides + ): + """Only constraint acceptances can be hard; nothing else is logged.""" + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + + with pytest.raises(ValueError, match="Severity"): + processor.apply_corrections( + alias="survey", + key_col="survey_key", + entries=[self._hard_accept(**overrides)], + source="outliers", + ) + + assert processor.get_correction_log("survey").is_empty() + + def test_legacy_logs_load_with_null_severity(self, store, sample_corrections_log): + store[("p1", "logs", "corr_log_survey")] = sample_corrections_log + + log = CorrectionProcessor("p1").get_correction_log("survey") + + assert log["severity"].to_list() == [None] * 3 + + def test_summary_carries_the_severity(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + processor.apply_corrections( + alias="survey", + key_col="survey_key", + entries=[self._hard_accept()], + source="constraints", + ) + + (summary,) = processor.get_correction_summary("survey") + + assert summary["severity"] == "hard" + + +class TestActiveCorrections: + """Value corrections whose result the data still holds.""" + + def _correct(self, processor, *entries): + processor.apply_corrections( + alias="survey", + key_col="survey_key", + entries=list(entries), + source="outliers", + ) + + def _modify(self, key, column, new_value, current_value, reason="typo"): + return CorrectionEntry( + key_value=key, + action="modify value", + column=column, + current_value=current_value, + new_value=new_value, + reason=reason, + ) + + def test_returns_modify_and_remove_value_corrections(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + self._correct( + processor, + self._modify("key1", "age", 26, 25), + CorrectionEntry( + key_value="key2", + action="remove value", + column="age", + current_value=30, + reason="impossible", + ), + ) + + active = processor.get_active_corrections("survey", "survey_key") + + assert active.select("KEY", "action", "column", "reason").rows() == [ + ("key1", "modify value", "age", "typo"), + ("key2", "remove value", "age", "impossible"), + ] + + def test_a_correction_overwritten_later_is_inactive(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + self._correct(processor, self._modify("key1", "age", 26, 25, reason="first")) + self._correct(processor, self._modify("key1", "age", 27, 26, reason="second")) + + active = processor.get_active_corrections("survey", "survey_key") + + assert active["reason"].to_list() == ["second"] + + def test_excludes_acceptances_and_removed_rows(self, store, sample_data): + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + self._correct( + processor, + CorrectionEntry( + key_value="key1", + action="accept", + check_type="outliers", + column="age", + current_value=25, + reason="ok", + ), + CorrectionEntry(key_value="key3", action="remove row", reason="dup"), + ) + + assert processor.get_active_corrections("survey", "survey_key").is_empty() + + def test_a_correction_that_failed_to_reapply_is_inactive(self, store, sample_data): + """Prep data that holds the new value doesn't make a failed correction + look applied. + """ + _seed_prep(store, sample_data) + processor = CorrectionProcessor("p1") + self._correct(processor, self._modify("key1", "age", 26, 25)) + # Prep now supplies 26 itself, so replay finds 26 where the + # correction expects 25 and rejects it. + _seed_prep( + store, + sample_data.with_columns( + pl.when(pl.col("survey_key") == "key1") + .then(26) + .otherwise(pl.col("age")) + .alias("age") + ), + ) + + failures = processor.refresh_existing_corrected_data("survey") + + assert len(failures) == 1 + assert processor.get_corrected_data("survey")["age"][0] == 26 + assert processor.get_active_corrections("survey", "survey_key").is_empty() + + def test_empty_log_returns_no_corrections(self, store, sample_data): + _seed_prep(store, sample_data) + + active = CorrectionProcessor("p1").get_active_corrections( + "survey", "survey_key" + ) + + assert active.is_empty() + + +class TestActiveCorrectionsMatchStoredValues: + """A correction is active if the cell holds the value it stored.""" + + @staticmethod + def _modify(key, new_value, current_value): + return CorrectionEntry( + key_value=key, + action="modify value", + column="age", + current_value=current_value, + new_value=new_value, + reason="typo", + ) + + def test_float32_modification_is_active(self, store): + """70.1 is stored as 70.0999984741211 in a Float32 column.""" + _seed_prep( + store, + pl.DataFrame( + {"KEY": ["a", "b"], "age": [150.0, 30.0]}, + schema={"KEY": pl.String, "age": pl.Float32}, + ), + ) + processor = CorrectionProcessor("p1") + processor.apply_corrections( + "survey", "KEY", [self._modify("a", "70.1", 150.0)], source="outliers" + ) + + active = processor.get_active_corrections("survey", "KEY") + + assert active["KEY"].to_list() == ["a"] + + def test_numeric_key_correction_applies_and_is_active(self, store): + _seed_prep(store, pl.DataFrame({"KEY": [7, 8], "age": [150, 30]})) + processor = CorrectionProcessor("p1") + + processor.apply_corrections( + "survey", "KEY", [self._modify(7, "90", 150)], source="outliers" + ) + + assert processor.get_corrected_data("survey")["age"].to_list() == [90, 30] + active = processor.get_active_corrections("survey", "KEY") + assert active["KEY"].to_list() == ["7"] + + def test_duplicate_keys_must_all_hold_the_value(self, store): + _seed_prep( + store, + pl.DataFrame({"KEY": ["a", "a", "b"], "age": [150, 150, 30]}), + ) + processor = CorrectionProcessor("p1") + processor.apply_corrections( + "survey", "KEY", [self._modify("a", "90", 150)], source="outliers" + ) + # Another step changes one of the duplicate rows afterwards. + data = processor.get_corrected_data("survey") + processor.save_corrected_data( + "survey", + data.with_columns( + pl.when(pl.int_range(pl.len()) == 1) + .then(pl.lit(91)) + .otherwise(pl.col("age")) + .alias("age") + ), + ) + + assert processor.get_active_corrections("survey", "KEY").is_empty() diff --git a/tests/replication/test_package_builder.py b/tests/replication/test_package_builder.py index 97804932..a68d6e49 100644 --- a/tests/replication/test_package_builder.py +++ b/tests/replication/test_package_builder.py @@ -342,7 +342,7 @@ def _duckdb_get(project_id, table, db_name): ) assert header == ( "date,KEY,ID,action,column,current_value,new_value,reason," - "status,status_reason,source,check_type" + "status,status_reason,source,check_type,severity" ) diff --git a/tests/utils/test_reapply_utils.py b/tests/utils/test_reapply_utils.py index 0acd0407..7d42257a 100644 --- a/tests/utils/test_reapply_utils.py +++ b/tests/utils/test_reapply_utils.py @@ -26,6 +26,13 @@ def test_successful_is_highlighted_green(self): def test_unknown_status_is_not_highlighted(self): assert highlight_status("") == "" + def test_missing_status_is_not_highlighted(self): + """A null status reaches a nullable-typed Styler as pd.NA.""" + import pandas as pd + + assert highlight_status(pd.NA) == "" + assert highlight_status(None) == "" + class TestWarnReapplyFailures: """Test the shared bulk-reapply warning banner helper.""" diff --git a/tests/utils/test_ui_utils.py b/tests/utils/test_ui_utils.py index bc84869f..9c5f5923 100644 --- a/tests/utils/test_ui_utils.py +++ b/tests/utils/test_ui_utils.py @@ -1,17 +1,23 @@ """Tests for the shared UI helpers module.""" import sys +import threading from unittest.mock import MagicMock +import pandas as pd +import polars as pl import pytest from datasure.utils.ui_utils import ( confirm_dialog, + ensure_styler_limit, metric_row, page_header, queue_notice, + row_styler, section_header, show_queued_notices, + styled_dataframe, ) @@ -222,8 +228,127 @@ def test_notices_are_scoped(self, st_with_state): st_with_state.success.assert_not_called() def test_showing_with_nothing_queued_is_a_no_op(self, st_with_state): - show_queued_notices("prep_survey") + shown = show_queued_notices("prep_survey") + assert shown is False st_with_state.success.assert_not_called() st_with_state.warning.assert_not_called() st_with_state.error.assert_not_called() + + def test_toast_notices_render_as_toasts(self, st_with_state): + queue_notice("outliers", "toast", "Saved") + + shown = show_queued_notices("outliers") + + assert shown is True + st_with_state.toast.assert_called_once_with("Saved") + + +@pytest.fixture +def small_limit(): + """Start from a low global Styler limit and restore it afterwards.""" + with pd.option_context("styler.render.max_elements", 10): + yield + + +class TestEnsureStylerLimit: + """The global Styler limit only ever goes up.""" + + def test_raises_a_lower_limit(self, small_limit): + ensure_styler_limit(100) + + assert pd.get_option("styler.render.max_elements") == 100 + + def test_never_lowers_the_limit(self, small_limit): + ensure_styler_limit(100) + ensure_styler_limit(20) + + assert pd.get_option("styler.render.max_elements") == 100 + + def test_concurrent_raises_keep_the_largest(self, small_limit): + sizes = list(range(11, 211)) + threads = [ + threading.Thread(target=ensure_styler_limit, args=(size,)) for size in sizes + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + assert pd.get_option("styler.render.max_elements") == max(sizes) + + +class TestStyledDataframe: + """Styled tables render whatever their size.""" + + def test_the_limit_fits_the_table_while_it_renders(self, mock_st, small_limit): + styler = pd.DataFrame({"a": range(50), "b": range(50)}).style + seen_limits = [] + mock_st.dataframe.side_effect = lambda *a, **k: seen_limits.append( + pd.get_option("styler.render.max_elements") + ) + + styled_dataframe(styler, width="stretch") + + assert seen_limits == [100] + mock_st.dataframe.assert_called_once_with(styler, width="stretch") + + def test_the_limit_is_not_lowered_afterwards(self, mock_st, small_limit): + """Restoring a lower limit could break a render in another session.""" + styled_dataframe(pd.DataFrame({"a": range(50)}).style) + + assert pd.get_option("styler.render.max_elements") == 50 + + def test_never_lowers_a_higher_limit(self, mock_st): + seen_limits = [] + mock_st.dataframe.side_effect = lambda *a, **k: seen_limits.append( + pd.get_option("styler.render.max_elements") + ) + + with pd.option_context("styler.render.max_elements", 1_000): + styled_dataframe(pd.DataFrame({"a": [1, 2]}).style) + + assert seen_limits == [1_000] + + def test_returns_what_st_dataframe_returns(self, mock_st): + result = styled_dataframe(pd.DataFrame({"a": [1]}).style) + + assert result is mock_st.dataframe.return_value + + +class TestRowStyler: + """Styling a table must not change how its values are displayed.""" + + @staticmethod + def _display_values(styler) -> list[list[str]]: + body = styler._translate(False, False)["body"] + return [[cell["display_value"] for cell in row[1:]] for row in body] + + def test_values_display_as_in_the_unstyled_table(self): + df = pl.DataFrame( + { + "KEY": ["K1", "K2"], + "age": [150, None], + "income": [1234.5678912, 70.5], + "note": ["x", None], + } + ) + + styler = row_styler(df, lambda row: [""] * len(row)) + + assert self._display_values(styler) == [ + ["K1", "150", "1234.5678912", "x"], + ["K2", "None", "70.5", "None"], + ] + + def test_applies_the_row_style(self): + df = pl.DataFrame({"KEY": ["K1", "K2"], "flag": ["yes", None]}) + + def green_if_flagged(row): + flagged = isinstance(row["flag"], str) + return ["background-color: green" if flagged else ""] * len(row) + + cell_styles = row_styler(df, green_if_flagged)._compute().ctx + + styled_rows = {row for (row, _), props in cell_styles.items() if props} + assert styled_rows == {0} diff --git a/tests/views/test_correction_view.py b/tests/views/test_correction_view.py index 94adeb02..f51f6ef7 100644 --- a/tests/views/test_correction_view.py +++ b/tests/views/test_correction_view.py @@ -5,6 +5,7 @@ from contextlib import contextmanager from unittest.mock import MagicMock, patch +import pandas as pd import polars as pl import pytest @@ -26,6 +27,7 @@ _handle_remove_correction, get_current_value, get_key_options, + highlight_hard_acceptance, load_hfc_config, load_tab_config, main, @@ -465,6 +467,7 @@ def test_status_columns_ordered_right_after_action(self): "status", "status_reason", "check_type", + "severity", "column", "current_value", "new_value", @@ -492,6 +495,46 @@ def test_accept_rows_show_their_check_type_and_source(self): ("accept", "outliers", "outliers") ] + def test_hard_violation_acceptances_show_their_severity(self): + log = self._base_log( + action=["accept"], + new_value=[None], + check_type=["constraints"], + source=["constraints"], + severity=["hard"], + ) + + result = _build_correction_log_display(log) + + assert result["severity"].to_list() == ["hard"] + + +class TestHighlightHardAcceptance: + """Hard-violation acceptances stand out in the Correction Log.""" + + def test_highlights_every_cell_of_a_hard_acceptance(self): + row = pd.Series({"action": "accept", "severity": "hard", "KEY": "k1"}) + + styles = highlight_hard_acceptance(row) + + assert len(styles) == len(row) + assert all(style and "background" in style for style in styles) + + @pytest.mark.parametrize( + ("action", "severity"), + [ + ("accept", None), + ("modify value", None), + ("accept", "soft"), + ("accept", pd.NA), + (pd.NA, pd.NA), + ], + ) + def test_leaves_other_rows_plain(self, action, severity): + row = pd.Series({"action": action, "severity": severity, "KEY": "k1"}) + + assert highlight_hard_acceptance(row) == ["", "", ""] + class TestLoadTabConfig: """Test that load_tab_config threads the configured Survey ID column."""