diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index 468c992..09e8c6b 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -24,11 +24,11 @@ jobs: - name: Install the project run: uv sync --locked --all-extras --dev - - name: Run Ruff (linting and formatting check) - run: uv run ruff check src/ tests/ + - name: Run Ruff (linting) + run: bash scripts/qa.sh lint - - name: Run Ruff (formatting check only) - run: uv run ruff format --check src/ tests/ + - name: Run Ruff (formatting check) + run: bash scripts/qa.sh format - name: Run tests - run: uv run pytest --cov=igh_data_transform --cov-report=term-missing + run: bash scripts/qa.sh test diff --git a/CLAUDE.md b/CLAUDE.md index 20c1018..b1173ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,6 +141,25 @@ Common UV commands: - **Run commands without activating venv**: `uv run ` - **Run Python scripts**: `uv run python ` +### QA (Lint + Format + Tests) + +```bash +# Run all QA checks (linting, format check, unit tests with coverage) +bash scripts/qa.sh + +# Run a single step +bash scripts/qa.sh lint +bash scripts/qa.sh format +bash scripts/qa.sh test + +# Pass extra pytest flags +bash scripts/qa.sh test -v --all +``` + +`scripts/qa.sh` is the single source of truth for QA steps — the GitHub Actions workflow calls the same script with individual step arguments, so local and CI checks cannot diverge. + +When `data/dataverse_complete_raw.db` exists (the default output of `sync-and-run-etl.sh`), the script automatically sets `E2E_BRONZE_DB_PATH` and runs e2e tests with `--all`. On CI (where no Bronze DB is present), only unit tests run. + ### Running Tests ```bash diff --git a/README.md b/README.md index 4781625..09382e3 100644 --- a/README.md +++ b/README.md @@ -139,11 +139,18 @@ The project uses UV for dependency management. Common commands: - **Add a dev dependency**: `uv add --dev ` - **Update dependencies**: `uv sync` - **Run commands without activating venv**: `uv run ` -- **Run unit tests**: `uv run pytest` -- **Run e2e tests**: `E2E_BRONZE_DB_PATH=/path/to/bronze.db uv run pytest --e2e -v` -- **Run all tests**: `E2E_BRONZE_DB_PATH=/path/to/bronze.db uv run pytest --all -v` -- **Run tests with coverage**: `uv run pytest --cov=igh_data_transform --cov-report=term-missing` -- **Run linter**: `uv run ruff check src/ tests/` +- **Run all QA checks** (lint + format + tests): `bash scripts/qa.sh` +- **Run a single QA step**: `bash scripts/qa.sh lint`, `bash scripts/qa.sh format`, `bash scripts/qa.sh test` +- **Pass extra pytest flags**: `bash scripts/qa.sh test -v --all` +- **Run e2e tests**: `E2E_BRONZE_DB_PATH=/path/to/bronze.db bash scripts/qa.sh test --e2e -v` + +The same `scripts/qa.sh` is used by the GitHub Actions CI workflow, so local and CI checks always match. + +If `data/dataverse_complete_raw.db` exists (the default output of `sync-and-run-etl.sh`), e2e tests are included automatically. You can also set `E2E_BRONZE_DB_PATH` explicitly to point at a different Bronze DB. + +### Special Case comments + +Code paths that work around known defects in the upstream CRM data are marked with a `# Special Case:` comment followed by an explanation on the next line. These are intended to be temporary — once the source data is corrected, search for `Special Case` to find and remove them. ### Documentation diff --git a/scripts/qa.sh b/scripts/qa.sh new file mode 100755 index 0000000..d0ec629 --- /dev/null +++ b/scripts/qa.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# QA checks: linting, formatting, and tests. +# The GitHub Actions workflow calls each step individually so they appear +# as separate checks, while `bash scripts/qa.sh` runs everything locally. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_DIR="$(dirname "$SCRIPT_DIR")" +DEFAULT_BRONZE_DB="$REPO_DIR/data/dataverse_complete_raw.db" + +# Auto-enable e2e tests when the default Bronze DB exists and +# E2E_BRONZE_DB_PATH hasn't been explicitly set. +if [ -z "${E2E_BRONZE_DB_PATH:-}" ] && [ -f "$DEFAULT_BRONZE_DB" ]; then + export E2E_BRONZE_DB_PATH="$DEFAULT_BRONZE_DB" +fi + +pytest_e2e_flags() { + if [ -n "${E2E_BRONZE_DB_PATH:-}" ]; then + echo "--all" + fi +} + +step_lint() { + echo "==> Ruff lint" + uv run ruff check src/ tests/ +} + +step_format() { + echo "==> Ruff format check" + uv run ruff format --check src/ tests/ +} + +step_test() { + echo "==> Tests" + # shellcheck disable=SC2046 + uv run pytest --cov=igh_data_transform --cov-report=term-missing \ + $(pytest_e2e_flags) "$@" +} + +case "${1:-all}" in + lint) step_lint ;; + format) step_format ;; + test) shift; step_test "$@" ;; + all) shift 2>/dev/null || true; step_lint && step_format && step_test "$@" ;; + *) echo "Usage: $0 {lint|format|test|all} [extra pytest args...]"; exit 1 ;; +esac diff --git a/src/igh_data_transform/transformations/candidates.py b/src/igh_data_transform/transformations/candidates.py index 5b0edd8..0b57f58 100644 --- a/src/igh_data_transform/transformations/candidates.py +++ b/src/igh_data_transform/transformations/candidates.py @@ -190,6 +190,7 @@ def _normalize_pipeline_cols(df: pd.DataFrame) -> pd.DataFrame: if col in df.columns: df[col] = df[col].replace(_PIPELINE_CODE_NORMALIZATION) + # Special Case: # 2019 candidates were not independently reviewed in 2021 — preserve # their pipeline inclusion by overriding 2021 "No" when 2019 is "Yes". _2019_col = "vin_2019pcrpipelineinclusion" diff --git a/src/igh_data_transform/transformations/diseases.py b/src/igh_data_transform/transformations/diseases.py index 7ce1571..e7c0eb9 100644 --- a/src/igh_data_transform/transformations/diseases.py +++ b/src/igh_data_transform/transformations/diseases.py @@ -154,6 +154,20 @@ def transform_diseases( other=pd.NA, ) + # Special Case: Filoviral diseases parent-level record missing disease_filter. + # Bronze disease "Filoviral diseases (including Ebola, Marburg)" (the parent + # umbrella record, diseaseid 13b20daf-d5a0-ec11-b400-002248185df1) has a NULL + # new_diseasefilter in the CRM source. The priority "TPP: multivalent filovirus + # vaccines" is linked to this record. Without a disease_filter the disease + # cannot appear in the hierarchical filter dropdown, making the priority + # unreachable. Backfill the filter to "Filoviral diseases" so it groups with + # its siblings. Remove this once the CRM source data is corrected. + if "disease_filter" in df.columns and "name" in df.columns: + filo_mask = df["name"].str.strip().str.startswith( + "Filoviral diseases (including Ebola, Marburg)", na=False + ) & (df["disease_filter"].isna() | (df["disease_filter"] == "")) + df.loc[filo_mask, "disease_filter"] = "Filoviral diseases" + # STI primary normalization (self-validating). # # Three rows in the current Bronze sample store the primary as diff --git a/tests/unit/test_diseases.py b/tests/unit/test_diseases.py index 42f811b..d40cc41 100644 --- a/tests/unit/test_diseases.py +++ b/tests/unit/test_diseases.py @@ -283,6 +283,38 @@ def test_inclusion_flag_columns_are_not_in_silver_output(self): assert "incl_nd" not in result.columns assert "incl_eid" not in result.columns + def test_backfills_filoviral_disease_filter(self): + # Special Case: the parent-level Filoviral record arrives from the + # CRM with a NULL disease_filter, making its linked priority + # unreachable in the filter dropdown. The transform backfills it. + df = self._make_input_df( + overrides={ + "vin_name": [ + "Filoviral diseases (including Ebola, Marburg) - Multiple filoviral diseases - Vaccines", + "Some other disease", + ], + "new_diseasefilter": [None, "Malaria"], + } + ) + result, _ = transform_diseases(df) + assert result["disease_filter"].iloc[0] == "Filoviral diseases" + # Other rows untouched. + assert result["disease_filter"].iloc[1] == "Malaria" + + def test_filoviral_backfill_does_not_overwrite_existing_filter(self): + # When a Filoviral row already has a disease_filter, leave it alone. + df = self._make_input_df( + overrides={ + "vin_name": [ + "Filoviral diseases (including Ebola, Marburg) - Ebola - Vaccines", + "Other", + ], + "new_diseasefilter": ["Filoviral diseases", None], + } + ) + result, _ = transform_diseases(df) + assert result["disease_filter"].iloc[0] == "Filoviral diseases" + def test_normalizes_sti_primary_when_suffix_matches_secondary(self): # Three Bronze rows store new_diseasefilter as a parent-child # concatenation. Collapse only when the suffix exactly matches