diff --git a/.github/find-api-model-versions.py b/.github/find-api-model-versions.py index 9ae2ab7d4..2f3843b58 100644 --- a/.github/find-api-model-versions.py +++ b/.github/find-api-model-versions.py @@ -9,6 +9,7 @@ import shlex import sys from importlib.metadata import version as distribution_version +from pathlib import Path from policyengine.bundle import get_current_bundle @@ -97,7 +98,7 @@ def find_api_model_versions() -> dict[str, str]: def find_api_model_versions_and_output_to_github() -> None: """Find the API model versions and output them to a file for GitHub.""" versions = find_api_model_versions() - with open(os.environ["GITHUB_ENV"], "a") as f: + with Path(os.environ["GITHUB_ENV"]).open("a") as f: for key, value in versions.items(): f.write(f"{key}={value}\n") diff --git a/AGENTS.md b/AGENTS.md index fd978b11c..2ff3f6da7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,7 +96,7 @@ To add or change a rule, read `docs/engineering/skills/rules-registry.md`. ### Code quality and types -- Checked: `api.quality.alembic-revisions-unused-imports`, `api.quality.ci-cd-scripts-ruff-exemptions`, `api.quality.command-line-modules-ruff-exemptions`, `api.quality.command-line-scripts-ruff-exemptions`, `api.quality.environment-variables-load`, `api.quality.mypy-v2`, `api.quality.ruff-format`, `api.quality.ruff-lint`, `api.quality.runtime-evaluated-models-resolve`, `api.quality.tests-pass`, `api.quality.tests-ruff-exemptions` +- Checked: `api.quality.alembic-revisions-unused-imports`, `api.quality.ci-cd-scripts-ruff-exemptions`, `api.quality.command-line-modules-ruff-exemptions`, `api.quality.command-line-scripts-ruff-exemptions`, `api.quality.environment-variables-load`, `api.quality.mypy-v2`, `api.quality.ruff-format`, `api.quality.ruff-lint`, `api.quality.runtime-evaluated-models-resolve`, `api.quality.tests-pass`, `api.quality.tests-ruff-exemptions`, `api.quality.unittest-ruff-exemptions` ### Observability diff --git a/changelog.d/ruff-idioms-arguments.changed.md b/changelog.d/ruff-idioms-arguments.changed.md new file mode 100644 index 000000000..36427a81d --- /dev/null +++ b/changelog.d/ruff-idioms-arguments.changed.md @@ -0,0 +1 @@ +Enabled ruff's argument, idiom and pytest-style rules (ARG, ASYNC, B, C4, FBT, PD, PERF, PGH, PLW, PT, PTH, RUF, SIM): unused arguments are removed, underscored or explained (test stand-ins that mirror a real signature take ARG001, ARG002 and ARG005 as file-level test exceptions instead), tests request side-effect fixtures with usefixtures and match the exception messages they expect, boolean options of internal helpers are keyword-only, and re-raised errors keep their cause; published schemas and the HTTP API are unchanged, and the shared rules tool's unittest tests are exempt from pytest-style rules. diff --git a/docs/generated/rules.md b/docs/generated/rules.md index 65e37b91b..5ab993844 100644 --- a/docs/generated/rules.md +++ b/docs/generated/rules.md @@ -8,11 +8,11 @@ Every rule that governs `policyengine-api` is listed here with how it is enforce | Tier | enforced | ratcheted | advisory | doc-only | Total | | --- | ---: | ---: | ---: | ---: | ---: | -| mechanical | 50 | 0 | 1 | 12 | 63 | +| mechanical | 51 | 0 | 1 | 12 | 64 | | metric | 0 | 0 | 0 | 0 | 0 | | judgment | 0 | 0 | 0 | 1 | 1 | -**Blocking:** 50 of 64 rules. +**Blocking:** 51 of 65 rules. ## Gaps @@ -296,10 +296,11 @@ flowchart LR | `api.quality.environment-variables-load` | Required environment variables and credentials load in a deployed-like environment. | mechanical | enforced | yes | CI job `test_env_vars` | `pr.yml:test_env_vars` | — | | `api.quality.mypy-v2` | Shared query models, FastAPI dependencies, v2 routes and services, and the user sign-in and account-deletion libraries pass mypy with untyped definitions disallowed. | mechanical | enforced | yes | mypy | `pr.yml:typecheck-v2` | — | | `api.quality.ruff-format` | All Python files are formatted with ruff format. | mechanical | enforced | yes | ruff format | `pr.yml:lint` | — | -| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (A, ANN, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TID253, TRY, UP, W) | `pr.yml:lint` | — | +| `api.quality.ruff-lint` | All Python code passes ruff check with every rule family selected in pyproject.toml, with no ratchet; families are added one themed group at a time until the selection is ALL. | mechanical | enforced | yes | ruff check (A, ANN, ARG, ASYNC, B, BLE, C4, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FBT, FIX, FLY, FURB, G, I, INP, LOG, N, PD, PERF, PGH, PIE, PLC, PLW, PT, PTH, PYI, RET, RSE, RUF, S, SIM, SLF, T20, TC, TD, TID253, TRY, UP, W) | `pr.yml:lint` | — | | `api.quality.runtime-evaluated-models-resolve` | Every package model is one ruff treats as runtime-evaluated (a base resolves, through the import in its own module, to a name in runtime-evaluated-base-classes, or is a same-module class that does), and every Pydantic model and SQLAlchemy mapper resolves its annotations once the package is imported. | mechanical | enforced | yes | pytest `tests/unit/test_runtime_evaluated_annotations.py` | `pr.yml:test` | **unproven** | | `api.quality.tests-pass` | The unit, contract and to-refactor test suites pass. | mechanical | enforced | yes | CI job `test` | `pr.yml:test` | — | -| `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415). Their fakes may name methods after the interface they replace, such as a gRPC stub's Export (N802). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), and may import the lazily loaded packages at module level (TID253). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, N802, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for `**/tests/**` | `pr.yml:lint` | — | +| `api.quality.tests-ruff-exemptions` | Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415). Their fakes may name methods after the interface they replace, such as a gRPC stub's Export (N802). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), may import the lazily loaded packages at module level (TID253), and may leave a stand-in's arguments unused (ARG001, ARG002, ARG005). Every other ruff rule applies to tests. | mechanical | enforced | yes | ruff per-file ignore of ANN, ARG001, ARG002, ARG005, D1, DTZ001, DTZ011, INP001, N802, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for `**/tests/**` | `pr.yml:lint` | — | +| `api.quality.unittest-ruff-exemptions` | The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them. | mechanical | enforced | yes | ruff per-file ignore of PT for `scripts/rules/tests/**` | `pr.yml:lint` | — | ```mermaid flowchart LR @@ -321,14 +322,16 @@ flowchart LR r_api_quality_ruff_format("api.quality.ruff-format (enforced)") e_ruff_format["ruff format"] r_api_quality_ruff_lint("api.quality.ruff-lint (enforced)") - e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TID253__TRY__UP__W_["ruff check (A, ANN, BLE, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FIX, FLY, FURB, G, I, INP, LOG, N, PIE, PLC, PYI, RET, RSE, S, SLF, T20, TC, TD, TID253, TRY, UP, W)"] + e_ruff_check__A__ANN__ARG__ASYNC__B__BLE__C4__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FBT__FIX__FLY__FURB__G__I__INP__LOG__N__PD__PERF__PGH__PIE__PLC__PLW__PT__PTH__PYI__RET__RSE__RUF__S__SIM__SLF__T20__TC__TD__TID253__TRY__UP__W_["ruff check (A, ANN, ARG, ASYNC, B, BLE, C4, D, DTZ, E, E4, E7, E9, EM, ERA, EXE, F, FAST, FBT, FIX, FLY, FURB, G, I, INP, LOG, N, PD, PERF, PGH, PIE, PLC, PLW, PT, PTH, PYI, RET, RSE, RUF, S, SIM, SLF, T20, TC, TD, TID253, TRY, UP, W)"] r_api_quality_runtime_evaluated_models_resolve("api.quality.runtime-evaluated-models-resolve (enforced)") e_pytest_tests_unit_test_runtime_evaluated_annotations_py["pytest tests/unit/test_runtime_evaluated_annotations.py"] j_CI_pr_yml_test["CI pr.yml:test"] r_api_quality_tests_pass("api.quality.tests-pass (enforced)") e_CI_job_test["CI job test"] r_api_quality_tests_ruff_exemptions("api.quality.tests-ruff-exemptions (enforced)") - e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___["ruff per-file ignore of ANN, D1, DTZ001, DTZ011, INP001, N802, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for **/tests/**"] + e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___["ruff per-file ignore of ANN, ARG001, ARG002, ARG005, D1, DTZ001, DTZ011, INP001, N802, PLC0415, PLR2004, S101, S105, S106, S603, S607, S608, SLF001, T201, TID253 for **/tests/**"] + r_api_quality_unittest_ruff_exemptions("api.quality.unittest-ruff-exemptions (enforced)") + e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___["ruff per-file ignore of PT for scripts/rules/tests/**"] r_api_quality_alembic_revisions_unused_imports -.-> gap_unenforced r_api_quality_ci_cd_scripts_ruff_exemptions --> e_ruff_per_file_ignore_of_N999__S310__T201__TID253_for__github___ e_ruff_per_file_ignore_of_N999__S310__T201__TID253_for__github___ --> j_CI_pr_yml_lint @@ -342,14 +345,16 @@ flowchart LR e_mypy --> j_CI_pr_yml_typecheck_v2 r_api_quality_ruff_format --> e_ruff_format e_ruff_format --> j_CI_pr_yml_lint - r_api_quality_ruff_lint --> e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TID253__TRY__UP__W_ - e_ruff_check__A__ANN__BLE__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FIX__FLY__FURB__G__I__INP__LOG__N__PIE__PLC__PYI__RET__RSE__S__SLF__T20__TC__TD__TID253__TRY__UP__W_ --> j_CI_pr_yml_lint + r_api_quality_ruff_lint --> e_ruff_check__A__ANN__ARG__ASYNC__B__BLE__C4__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FBT__FIX__FLY__FURB__G__I__INP__LOG__N__PD__PERF__PGH__PIE__PLC__PLW__PT__PTH__PYI__RET__RSE__RUF__S__SIM__SLF__T20__TC__TD__TID253__TRY__UP__W_ + e_ruff_check__A__ANN__ARG__ASYNC__B__BLE__C4__D__DTZ__E__E4__E7__E9__EM__ERA__EXE__F__FAST__FBT__FIX__FLY__FURB__G__I__INP__LOG__N__PD__PERF__PGH__PIE__PLC__PLW__PT__PTH__PYI__RET__RSE__RUF__S__SIM__SLF__T20__TC__TD__TID253__TRY__UP__W_ --> j_CI_pr_yml_lint r_api_quality_runtime_evaluated_models_resolve --> e_pytest_tests_unit_test_runtime_evaluated_annotations_py e_pytest_tests_unit_test_runtime_evaluated_annotations_py --> j_CI_pr_yml_test r_api_quality_tests_pass --> e_CI_job_test e_CI_job_test --> j_CI_pr_yml_test - r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ - e_ruff_per_file_ignore_of_ANN__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ --> j_CI_pr_yml_lint + r_api_quality_tests_ruff_exemptions --> e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ + e_ruff_per_file_ignore_of_ANN__ARG001__ARG002__ARG005__D1__DTZ001__DTZ011__INP001__N802__PLC0415__PLR2004__S101__S105__S106__S603__S607__S608__SLF001__T201__TID253_for____tests___ --> j_CI_pr_yml_lint + r_api_quality_unittest_ruff_exemptions --> e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___ + e_ruff_per_file_ignore_of_PT_for_scripts_rules_tests___ --> j_CI_pr_yml_lint ``` ## Observability diff --git a/migrations/v2/env.py b/migrations/v2/env.py index b1b88bb38..6f84f08c3 100644 --- a/migrations/v2/env.py +++ b/migrations/v2/env.py @@ -34,7 +34,7 @@ def _include_application_object( _object: SchemaItem, name: str | None, type_: NameFilterType, - _reflected: bool, + _reflected: bool, # noqa: FBT001 # Alembic calls include_object with positional arguments _compare_to: SchemaItem | None, ) -> bool: """Keep Alembic's own version table outside application drift.""" diff --git a/policyengine_api/constants.py b/policyengine_api/constants.py index 132aff38f..49b425f91 100644 --- a/policyengine_api/constants.py +++ b/policyengine_api/constants.py @@ -59,7 +59,7 @@ def get_py_manifest() -> Any: # noqa: ANN401 # SimplePath, which open() reject def _load_policyengine_bundle() -> dict | None: try: - with open(get_py_manifest()) as manifest_file: + with Path(get_py_manifest()).open() as manifest_file: manifest = json.load(manifest_file) except Exception as exc: msg = ( @@ -167,11 +167,11 @@ def get_bundle_default_dataset_option(country_id: str) -> dict: } COUNTRY_PACKAGE_VERSIONS = { country: _resolve_distribution_version(_dist_versions, package_name) - for country, package_name in zip(COUNTRIES, COUNTRY_PACKAGE_NAMES) + for country, package_name in zip(COUNTRIES, COUNTRY_PACKAGE_NAMES, strict=True) } except Exception: # noqa: BLE001 # unreadable package metadata falls back to 0.0.0; import must not fail _dist_versions = {} - COUNTRY_PACKAGE_VERSIONS = {country: "0.0.0" for country in COUNTRIES} + COUNTRY_PACKAGE_VERSIONS = dict.fromkeys(COUNTRIES, "0.0.0") _policyengine_bundle = _load_policyengine_bundle() _bundle_package_versions = _resolve_bundle_package_versions(_policyengine_bundle) diff --git a/policyengine_api/country.py b/policyengine_api/country.py index 94aa6ff6a..caeb5ff07 100644 --- a/policyengine_api/country.py +++ b/policyengine_api/country.py @@ -1,5 +1,6 @@ """Loaded country tax-benefit systems, their metadata and household calculation.""" +import contextlib import importlib import inspect import json @@ -138,26 +139,26 @@ def __init__(self, country_package_name: str, country_id: str) -> None: def build_metadata(self) -> None: """Build the JSON-safe metadata document served by the metadata endpoints.""" self.metadata = self._json_safe( - dict( - variables=self.build_variables(), - parameters=self.build_parameters(), - entities=self.build_entities(), - variableModules=self.tax_benefit_system.variable_module_metadata, - economy_options=self.build_microsimulation_options(), - current_law_id={ + { + "variables": self.build_variables(), + "parameters": self.build_parameters(), + "entities": self.build_entities(), + "variableModules": self.tax_benefit_system.variable_module_metadata, + "economy_options": self.build_microsimulation_options(), + "current_law_id": { "uk": 1, "us": 2, "ca": 3, "ng": 4, "il": 5, }[self.country_id], - basicInputs=self.tax_benefit_system.basic_inputs, - modelled_policies=self.tax_benefit_system.modelled_policies, - spm=spm_metadata(self.country_id), - version=get_package_version( + "basicInputs": self.tax_benefit_system.basic_inputs, + "modelled_policies": self.tax_benefit_system.modelled_policies, + "spm": spm_metadata(self.country_id), + "version": get_package_version( self.country_package_name.replace("_", "-") ), - ) + } ) def _json_safe( @@ -181,7 +182,7 @@ def build_microsimulation_options(self) -> dict: """Build the regions, time periods and datasets offered for economy runs.""" # Shape: { region: [{ name: "uk", label: "the UK" }], # time_period: [{ name: 2022, label: "2022", ... }] } - options = dict() + options = {} if self.country_id == "uk": constituency_names_path = ( Path(__file__).parent / "data" / "constituencies_2024.csv" @@ -189,41 +190,41 @@ def build_microsimulation_options(self) -> dict: constituency_names = pd.read_csv(constituency_names_path) constituency_names = constituency_names.sort_values("name") region = [ - dict(name="uk", label="the UK", type="national"), - dict(name="country/england", label="England", type="country"), - dict(name="country/scotland", label="Scotland", type="country"), - dict(name="country/wales", label="Wales", type="country"), - dict(name="country/ni", label="Northern Ireland", type="country"), + {"name": "uk", "label": "the UK", "type": "national"}, + {"name": "country/england", "label": "England", "type": "country"}, + {"name": "country/scotland", "label": "Scotland", "type": "country"}, + {"name": "country/wales", "label": "Wales", "type": "country"}, + {"name": "country/ni", "label": "Northern Ireland", "type": "country"}, ] - for i in range(len(constituency_names)): - region.append( - dict( - name=f"constituency/{constituency_names.iloc[i]['name']}", - label=constituency_names.iloc[i]["name"], - type="constituency", - ) - ) + region.extend( + { + "name": f"constituency/{constituency_names.iloc[i]['name']}", + "label": constituency_names.iloc[i]["name"], + "type": "constituency", + } + for i in range(len(constituency_names)) + ) local_authority_names_path = ( Path(__file__).parent / "data" / "local_authorities_2021.csv" ) local_authority_names = pd.read_csv(local_authority_names_path) local_authority_names = local_authority_names.sort_values("name") - for i in range(len(local_authority_names)): - region.append( - dict( - name=f"local_authority/{local_authority_names.iloc[i]['name']}", - label=local_authority_names.iloc[i]["name"], - type="local_authority", - ) - ) + region.extend( + { + "name": f"local_authority/{local_authority_names.iloc[i]['name']}", + "label": local_authority_names.iloc[i]["name"], + "type": "local_authority", + } + for i in range(len(local_authority_names)) + ) time_period = [ - dict(name=2024, label="2024"), - dict(name=2025, label="2025"), - dict(name=2026, label="2026"), - dict(name=2027, label="2027"), - dict(name=2028, label="2028"), - dict(name=2029, label="2029"), - dict(name=2030, label="2030"), + {"name": 2024, "label": "2024"}, + {"name": 2025, "label": "2025"}, + {"name": 2026, "label": "2026"}, + {"name": 2027, "label": "2027"}, + {"name": 2028, "label": "2028"}, + {"name": 2029, "label": "2029"}, + {"name": 2030, "label": "2030"}, ] datasets = [get_bundle_default_dataset_option("uk")] options["region"] = region @@ -231,76 +232,76 @@ def build_microsimulation_options(self) -> dict: options["datasets"] = datasets elif self.country_id == "us": region = [ - dict(name="us", label="the US", type="national"), - dict(name="state/al", label="Alabama", type="state"), - dict(name="state/ak", label="Alaska", type="state"), - dict(name="state/az", label="Arizona", type="state"), - dict(name="state/ar", label="Arkansas", type="state"), - dict(name="state/ca", label="California", type="state"), - dict(name="state/co", label="Colorado", type="state"), - dict(name="state/ct", label="Connecticut", type="state"), - dict(name="state/de", label="Delaware", type="state"), - dict(name="state/dc", label="District of Columbia", type="state"), - dict(name="state/fl", label="Florida", type="state"), - dict(name="state/ga", label="Georgia", type="state"), - dict(name="state/hi", label="Hawaii", type="state"), - dict(name="state/id", label="Idaho", type="state"), - dict(name="state/il", label="Illinois", type="state"), - dict(name="state/in", label="Indiana", type="state"), - dict(name="state/ia", label="Iowa", type="state"), - dict(name="state/ks", label="Kansas", type="state"), - dict(name="state/ky", label="Kentucky", type="state"), - dict(name="state/la", label="Louisiana", type="state"), - dict(name="state/me", label="Maine", type="state"), - dict(name="state/md", label="Maryland", type="state"), - dict(name="state/ma", label="Massachusetts", type="state"), - dict(name="state/mi", label="Michigan", type="state"), - dict(name="state/mn", label="Minnesota", type="state"), - dict(name="state/ms", label="Mississippi", type="state"), - dict(name="state/mo", label="Missouri", type="state"), - dict(name="state/mt", label="Montana", type="state"), - dict(name="state/ne", label="Nebraska", type="state"), - dict(name="state/nv", label="Nevada", type="state"), - dict(name="state/nh", label="New Hampshire", type="state"), - dict(name="state/nj", label="New Jersey", type="state"), - dict(name="state/nm", label="New Mexico", type="state"), - dict(name="state/ny", label="New York", type="state"), - dict(name="state/nc", label="North Carolina", type="state"), - dict(name="state/nd", label="North Dakota", type="state"), - dict(name="state/oh", label="Ohio", type="state"), - dict(name="state/ok", label="Oklahoma", type="state"), - dict(name="state/or", label="Oregon", type="state"), - dict(name="state/pa", label="Pennsylvania", type="state"), - dict(name="state/ri", label="Rhode Island", type="state"), - dict(name="state/sc", label="South Carolina", type="state"), - dict(name="state/sd", label="South Dakota", type="state"), - dict(name="state/tn", label="Tennessee", type="state"), - dict(name="state/tx", label="Texas", type="state"), - dict(name="state/ut", label="Utah", type="state"), - dict(name="state/vt", label="Vermont", type="state"), - dict(name="state/va", label="Virginia", type="state"), - dict(name="state/wa", label="Washington", type="state"), - dict(name="state/wv", label="West Virginia", type="state"), - dict(name="state/wi", label="Wisconsin", type="state"), - dict(name="state/wy", label="Wyoming", type="state"), + {"name": "us", "label": "the US", "type": "national"}, + {"name": "state/al", "label": "Alabama", "type": "state"}, + {"name": "state/ak", "label": "Alaska", "type": "state"}, + {"name": "state/az", "label": "Arizona", "type": "state"}, + {"name": "state/ar", "label": "Arkansas", "type": "state"}, + {"name": "state/ca", "label": "California", "type": "state"}, + {"name": "state/co", "label": "Colorado", "type": "state"}, + {"name": "state/ct", "label": "Connecticut", "type": "state"}, + {"name": "state/de", "label": "Delaware", "type": "state"}, + {"name": "state/dc", "label": "District of Columbia", "type": "state"}, + {"name": "state/fl", "label": "Florida", "type": "state"}, + {"name": "state/ga", "label": "Georgia", "type": "state"}, + {"name": "state/hi", "label": "Hawaii", "type": "state"}, + {"name": "state/id", "label": "Idaho", "type": "state"}, + {"name": "state/il", "label": "Illinois", "type": "state"}, + {"name": "state/in", "label": "Indiana", "type": "state"}, + {"name": "state/ia", "label": "Iowa", "type": "state"}, + {"name": "state/ks", "label": "Kansas", "type": "state"}, + {"name": "state/ky", "label": "Kentucky", "type": "state"}, + {"name": "state/la", "label": "Louisiana", "type": "state"}, + {"name": "state/me", "label": "Maine", "type": "state"}, + {"name": "state/md", "label": "Maryland", "type": "state"}, + {"name": "state/ma", "label": "Massachusetts", "type": "state"}, + {"name": "state/mi", "label": "Michigan", "type": "state"}, + {"name": "state/mn", "label": "Minnesota", "type": "state"}, + {"name": "state/ms", "label": "Mississippi", "type": "state"}, + {"name": "state/mo", "label": "Missouri", "type": "state"}, + {"name": "state/mt", "label": "Montana", "type": "state"}, + {"name": "state/ne", "label": "Nebraska", "type": "state"}, + {"name": "state/nv", "label": "Nevada", "type": "state"}, + {"name": "state/nh", "label": "New Hampshire", "type": "state"}, + {"name": "state/nj", "label": "New Jersey", "type": "state"}, + {"name": "state/nm", "label": "New Mexico", "type": "state"}, + {"name": "state/ny", "label": "New York", "type": "state"}, + {"name": "state/nc", "label": "North Carolina", "type": "state"}, + {"name": "state/nd", "label": "North Dakota", "type": "state"}, + {"name": "state/oh", "label": "Ohio", "type": "state"}, + {"name": "state/ok", "label": "Oklahoma", "type": "state"}, + {"name": "state/or", "label": "Oregon", "type": "state"}, + {"name": "state/pa", "label": "Pennsylvania", "type": "state"}, + {"name": "state/ri", "label": "Rhode Island", "type": "state"}, + {"name": "state/sc", "label": "South Carolina", "type": "state"}, + {"name": "state/sd", "label": "South Dakota", "type": "state"}, + {"name": "state/tn", "label": "Tennessee", "type": "state"}, + {"name": "state/tx", "label": "Texas", "type": "state"}, + {"name": "state/ut", "label": "Utah", "type": "state"}, + {"name": "state/vt", "label": "Vermont", "type": "state"}, + {"name": "state/va", "label": "Virginia", "type": "state"}, + {"name": "state/wa", "label": "Washington", "type": "state"}, + {"name": "state/wv", "label": "West Virginia", "type": "state"}, + {"name": "state/wi", "label": "Wisconsin", "type": "state"}, + {"name": "state/wy", "label": "Wyoming", "type": "state"}, ] # Add all 436 congressional districts (435 voting + DC) region.extend(build_congressional_district_metadata()) time_period = [ - dict(name=2035, label="2035"), - dict(name=2034, label="2034"), - dict(name=2033, label="2033"), - dict(name=2032, label="2032"), - dict(name=2031, label="2031"), - dict(name=2030, label="2030"), - dict(name=2029, label="2029"), - dict(name=2028, label="2028"), - dict(name=2027, label="2027"), - dict(name=2026, label="2026"), - dict(name=2025, label="2025"), - dict(name=2024, label="2024"), - dict(name=2023, label="2023"), - dict(name=2022, label="2022"), + {"name": 2035, "label": "2035"}, + {"name": 2034, "label": "2034"}, + {"name": 2033, "label": "2033"}, + {"name": 2032, "label": "2032"}, + {"name": 2031, "label": "2031"}, + {"name": 2030, "label": "2030"}, + {"name": 2029, "label": "2029"}, + {"name": 2028, "label": "2028"}, + {"name": 2027, "label": "2027"}, + {"name": 2026, "label": "2026"}, + {"name": 2025, "label": "2025"}, + {"name": 2024, "label": "2024"}, + {"name": 2023, "label": "2023"}, + {"name": 2022, "label": "2022"}, ] datasets = [get_bundle_default_dataset_option("us")] options["region"] = region @@ -308,10 +309,10 @@ def build_microsimulation_options(self) -> dict: options["datasets"] = datasets elif self.country_id == "ca": region = [ - dict(name="ca", label="Canada"), + {"name": "ca", "label": "Canada"}, ] time_period = [ - dict(name=2023, label="2023"), + {"name": 2023, "label": "2023"}, ] datasets = [{}] options["region"] = region @@ -319,10 +320,10 @@ def build_microsimulation_options(self) -> dict: options["datasets"] = datasets elif self.country_id == "ng": region = [ - dict(name="ng", label="Nigeria"), + {"name": "ng", "label": "Nigeria"}, ] time_period = [ - dict(name=2023, label="2023"), + {"name": 2023, "label": "2023"}, ] datasets = [{}] options["region"] = region @@ -330,10 +331,10 @@ def build_microsimulation_options(self) -> dict: options["datasets"] = datasets elif self.country_id == "il": region = [ - dict(name="il", label="Israel"), + {"name": "il", "label": "Israel"}, ] time_period = [ - dict(name=2023, label="2023"), + {"name": 2023, "label": "2023"}, ] datasets = [{}] options["region"] = region @@ -369,7 +370,7 @@ def build_variables(self) -> dict: } if variable.value_type.__name__ == "Enum": variable_data[variable_name]["possibleValues"] = [ - dict(value=value.name, label=value.value) + {"value": value.name, "label": value.value} for value in variable.possible_values ] variable_data[variable_name]["defaultValue"] = ( @@ -382,7 +383,7 @@ def build_parameters(self) -> dict: parameters = self.tax_benefit_system.parameters parameter_data = {} for parameter in parameters.get_descendants(): - if "gov" != parameter.name[:3]: + if parameter.name[:3] != "gov": continue end_name = parameter.name.split(".")[-1] if isinstance(parameter, ParameterScale): @@ -468,6 +469,7 @@ def calculate( household: dict, reform: dict | None, spm: dict | None = None, + *, spm_requested: bool = False, prepared: PreparedCountryCalculation | None = None, ) -> CalculationResult: @@ -506,7 +508,7 @@ def calculate( if has_axes: count_entities = len(household[entity_plural]) entity_index = 0 - for _entity_id in household[entity_plural].keys(): + for _entity_id in household[entity_plural]: if _entity_id == entity_id: break entity_index += 1 @@ -566,6 +568,7 @@ def prepare_calculation( household: dict, reform: dict | None, spm: dict | None = None, + *, spm_requested: bool = False, ) -> PreparedCountryCalculation: """Parse a situation before the API accepts it as a calculation.""" @@ -635,11 +638,12 @@ def _normalize_reform_values( node_type = type(parameter.values_list[-1].value) if node_type is int: node_type = float - try: - value = float(value) - except (TypeError, ValueError, OverflowError): - pass - normalized_reform[parameter_name][time_period] = node_type(value) + numeric_value = value + with contextlib.suppress(TypeError, ValueError, OverflowError): + numeric_value = float(value) + normalized_reform[parameter_name][time_period] = node_type( + numeric_value + ) return normalized_reform def _apply_reform_to_system( @@ -674,9 +678,9 @@ def modify_parameters(parameters: ParameterNode) -> ParameterNode: node = parameters for step in path.split("."): if "[" in step: - step, index = step.split("[") + child_name, index = step.split("[") index = int(index[:-1]) - node = node.children[step].brackets[index] + node = node.children[child_name].brackets[index] else: node = node.children[step] for period, value in values.items(): diff --git a/policyengine_api/data/congressional_districts.py b/policyengine_api/data/congressional_districts.py index 5c6603260..7f38f0e1d 100644 --- a/policyengine_api/data/congressional_districts.py +++ b/policyengine_api/data/congressional_districts.py @@ -699,7 +699,7 @@ def get_valid_state_codes() -> set[str]: Set of 51 lowercase state codes (50 states + DC) """ - return {code.lower() for code in STATE_CODE_TO_NAME.keys()} + return {code.lower() for code in STATE_CODE_TO_NAME} def get_valid_congressional_districts() -> set[str]: diff --git a/policyengine_api/data/orm.py b/policyengine_api/data/orm.py index 12e9a5ec2..8342eb3b7 100644 --- a/policyengine_api/data/orm.py +++ b/policyengine_api/data/orm.py @@ -4,6 +4,7 @@ import atexit import os +from dataclasses import dataclass from typing import TYPE_CHECKING from dotenv import load_dotenv @@ -23,9 +24,21 @@ DATABASE_POOL_SIZE = 5 DATABASE_POOL_MAX_OVERFLOW = 2 DATABASE_POOL_TIMEOUT_SECONDS = 30 -_v1_engine: Engine | None = None -_v1_session_factory: sessionmaker[Session] | None = None -_cloud_sql_connector: Connector | None = None + + +@dataclass +class _CloudSqlState: + """The process-owned Cloud SQL connector, engine and session factory. + + Each is created on first use and released by close_v1_engines. + """ + + connector: Connector | None = None + engine: Engine | None = None + session_factory: sessionmaker[Session] | None = None + + +_cloud_sql_state = _CloudSqlState() def get_remote_database_config() -> dict[str, str]: @@ -49,7 +62,6 @@ def build_session_factory(engine: Engine) -> sessionmaker[Session]: def _build_remote_engine() -> Engine: - global _cloud_sql_connector config = get_remote_database_config() connector = Connector( ip_type=CLOUD_SQL_IP_TYPE, @@ -75,42 +87,38 @@ def get_connection() -> DBAPIConnection: max_overflow=DATABASE_POOL_MAX_OVERFLOW, pool_timeout=DATABASE_POOL_TIMEOUT_SECONDS, ) - _cloud_sql_connector = connector + _cloud_sql_state.connector = connector return engine def get_v1_engine() -> Engine: """Return the process-owned Cloud SQL engine; never select SQLite.""" - global _v1_engine - if _v1_engine is None: - _v1_engine = _build_remote_engine() - return _v1_engine + if _cloud_sql_state.engine is None: + _cloud_sql_state.engine = _build_remote_engine() + return _cloud_sql_state.engine def get_v1_session_factory() -> sessionmaker[Session]: """Return one configured Session factory for the Cloud SQL engine.""" - global _v1_session_factory - if _v1_session_factory is None: - _v1_session_factory = build_session_factory(get_v1_engine()) - return _v1_session_factory + if _cloud_sql_state.session_factory is None: + _cloud_sql_state.session_factory = build_session_factory(get_v1_engine()) + return _cloud_sql_state.session_factory def clear_v1_session_factories() -> None: """Forget cached factories, primarily after replacing an Engine in tests.""" - global _v1_session_factory - _v1_session_factory = None + _cloud_sql_state.session_factory = None def close_v1_engines() -> None: """Release process-owned SQLAlchemy pools and Cloud SQL connectors.""" - global _cloud_sql_connector, _v1_engine clear_v1_session_factories() - if _v1_engine is not None: - _v1_engine.dispose() - _v1_engine = None - if _cloud_sql_connector is not None: - _cloud_sql_connector.close() - _cloud_sql_connector = None + if _cloud_sql_state.engine is not None: + _cloud_sql_state.engine.dispose() + _cloud_sql_state.engine = None + if _cloud_sql_state.connector is not None: + _cloud_sql_state.connector.close() + _cloud_sql_state.connector = None atexit.register(close_v1_engines) diff --git a/policyengine_api/data/v2/catalog/extraction.py b/policyengine_api/data/v2/catalog/extraction.py index 1a09f2abb..a21e8f7cc 100644 --- a/policyengine_api/data/v2/catalog/extraction.py +++ b/policyengine_api/data/v2/catalog/extraction.py @@ -611,7 +611,7 @@ def _normalize_country( regions: list[RegionRecord] = [] for source_region in source_regions: - code = str(getattr(source_region, "code")) + code = str(source_region.code) strategy = getattr(source_region, "scoping_strategy", None) requires_filter = bool(getattr(source_region, "requires_filter", False)) if requires_filter: @@ -649,7 +649,7 @@ def _normalize_country( field_name=f"region {code} label", maximum=255, ), - region_type=str(getattr(source_region, "region_type")), + region_type=str(source_region.region_type), requires_filter=requires_filter, filter_field=filter_field, filter_value=filter_value, diff --git a/policyengine_api/data/v2/catalog/publication_reconciliation.py b/policyengine_api/data/v2/catalog/publication_reconciliation.py index 98872c630..0977676c6 100644 --- a/policyengine_api/data/v2/catalog/publication_reconciliation.py +++ b/policyengine_api/data/v2/catalog/publication_reconciliation.py @@ -201,7 +201,7 @@ def _comparison_pairs( STAGE_CATALOG_DATASETS.c.name, STAGE_CATALOG_DATASETS.c.description, STAGE_CATALOG_DATASETS.c.year, - sa.literal(False), + sa.literal(value=False), sa.cast(sa.null(), DATASETS.c.storage_path.type), ).where(STAGE_CATALOG_DATASETS.c.country_id == country.country_id) actual_datasets = ( @@ -362,7 +362,7 @@ def assert_country_matches( STAGE_CATALOG_DATASETS.c.description, sa.cast(sa.null(), DATASETS.c.storage_path.type), STAGE_CATALOG_DATASETS.c.year, - sa.literal(False), + sa.literal(value=False), MODEL_VERSIONS.c.id, ) .select_from( diff --git a/policyengine_api/data/v2/catalog/publication_staging.py b/policyengine_api/data/v2/catalog/publication_staging.py index cd9b4675d..7f991b25a 100644 --- a/policyengine_api/data/v2/catalog/publication_staging.py +++ b/policyengine_api/data/v2/catalog/publication_staging.py @@ -281,11 +281,10 @@ def copy_rows( sql.SQL(", ").join(sql.Identifier(column.name) for column in table.columns), ) count = 0 - with raw_connection.cursor() as cursor: - with cursor.copy(statement) as copy: - for row in rows: - copy.write_row(row) - count += 1 + with raw_connection.cursor() as cursor, cursor.copy(statement) as copy: + for row in rows: + copy.write_row(row) + count += 1 return count diff --git a/policyengine_api/data/v2/database.py b/policyengine_api/data/v2/database.py index c5e7b5d0f..07b88ad1a 100644 --- a/policyengine_api/data/v2/database.py +++ b/policyengine_api/data/v2/database.py @@ -4,6 +4,7 @@ import atexit import os +from dataclasses import dataclass from threading import Lock from typing import TYPE_CHECKING @@ -24,11 +25,24 @@ DATABASE_POOL_MAX_OVERFLOW = 5 DATABASE_TIMEOUT_SECONDS = 5 + +@dataclass +class _V2EngineState: + """The process-owned v2 engine and Session factory. + + ``engine_pid`` is the ID of the process that created the engine, and + ``engine_url_fingerprint`` identifies its target. ``_state_lock`` guards + every field, and a forked child discards the state it inherits. + """ + + engine: Engine | None = None + engine_pid: int | None = None + engine_url_fingerprint: tuple[object, ...] | None = None + session_factory: sessionmaker[Session] | None = None + + _state_lock = Lock() -_engine: Engine | None = None -_engine_pid: int | None = None -_engine_url_fingerprint: tuple[object, ...] | None = None -_session_factory: sessionmaker[Session] | None = None +_engine_state = _V2EngineState() def _url_fingerprint(settings: V2DatabaseSettings) -> tuple[object, ...]: @@ -70,66 +84,61 @@ def build_v2_session_factory(engine: Engine) -> sessionmaker[Session]: def _discard_inherited_state(current_pid: int) -> None: """Drop pool state inherited through a process fork without touching parent FDs.""" - global _engine, _engine_pid, _engine_url_fingerprint, _session_factory - - if _engine is not None: - _engine.dispose(close=False) - _engine = None - _engine_pid = current_pid - _engine_url_fingerprint = None - _session_factory = None + if _engine_state.engine is not None: + _engine_state.engine.dispose(close=False) + _engine_state.engine = None + _engine_state.engine_pid = current_pid + _engine_state.engine_url_fingerprint = None + _engine_state.session_factory = None def get_v2_engine(settings: V2DatabaseSettings | None = None) -> Engine: """Return the one lazy v2 runtime engine owned by this process.""" - global _engine, _engine_pid, _engine_url_fingerprint - current_pid = os.getpid() with _state_lock: - if _engine_pid is not None and _engine_pid != current_pid: + if ( + _engine_state.engine_pid is not None + and _engine_state.engine_pid != current_pid + ): _discard_inherited_state(current_pid) selected_settings = settings or load_v2_runtime_database_settings() fingerprint = _url_fingerprint(selected_settings) - if _engine is not None: - if fingerprint != _engine_url_fingerprint: + if _engine_state.engine is not None: + if fingerprint != _engine_state.engine_url_fingerprint: msg = ( "the process-owned v2 engine is already bound to a different " "explicit target" ) raise V2ConfigurationError(msg) - return _engine + return _engine_state.engine - _engine = build_v2_engine(selected_settings) - _engine_pid = current_pid - _engine_url_fingerprint = fingerprint - return _engine + _engine_state.engine = build_v2_engine(selected_settings) + _engine_state.engine_pid = current_pid + _engine_state.engine_url_fingerprint = fingerprint + return _engine_state.engine def get_v2_session_factory( settings: V2DatabaseSettings | None = None, ) -> sessionmaker[Session]: """Return the process-owned SQLModel Session factory without opening a session.""" - global _session_factory - engine = get_v2_engine(settings) with _state_lock: - if _session_factory is None: - _session_factory = build_v2_session_factory(engine) - return _session_factory + if _engine_state.session_factory is None: + _engine_state.session_factory = build_v2_session_factory(engine) + return _engine_state.session_factory def close_v2_database() -> None: """Dispose this process's v2 pool and forget its lazy configuration.""" - global _engine, _engine_pid, _engine_url_fingerprint, _session_factory - with _state_lock: - if _engine is not None: - _engine.dispose() - _engine = None - _engine_pid = None - _engine_url_fingerprint = None - _session_factory = None + if _engine_state.engine is not None: + _engine_state.engine.dispose() + _engine_state.engine = None + _engine_state.engine_pid = None + _engine_state.engine_url_fingerprint = None + _engine_state.session_factory = None atexit.register(close_v2_database) diff --git a/policyengine_api/data/v2/models/__init__.py b/policyengine_api/data/v2/models/__init__.py index b8c7ed386..c63655bb6 100644 --- a/policyengine_api/data/v2/models/__init__.py +++ b/policyengine_api/data/v2/models/__init__.py @@ -95,6 +95,7 @@ validate_v2_metadata_table_names(V2_METADATA.tables) __all__ = [ + "V2_METADATA", "AggregateOutput", "AggregateType", "BudgetSummary", @@ -111,11 +112,11 @@ "HouseholdJobStatus", "Inequality", "IntraDecileImpact", - "LocalAuthorityImpact", - "LegacyPolicyMapping", "LegacyHouseholdMapping", + "LegacyPolicyMapping", "LegacyUserMapping", "LegacyUserPolicyMapping", + "LocalAuthorityImpact", "OutputStatus", "Parameter", "ParameterNode", @@ -147,6 +148,5 @@ "UserReportAssociation", "UserSimulationAssociation", "UserType", - "V2_METADATA", "Variable", ] diff --git a/policyengine_api/data/v2/privilege_reconciliation.py b/policyengine_api/data/v2/privilege_reconciliation.py index 29c74c624..d9d0cacec 100644 --- a/policyengine_api/data/v2/privilege_reconciliation.py +++ b/policyengine_api/data/v2/privilege_reconciliation.py @@ -28,6 +28,7 @@ import sys from collections import defaultdict from dataclasses import dataclass +from pathlib import Path from typing import TYPE_CHECKING from sqlalchemy import bindparam, text @@ -433,7 +434,7 @@ def _write_summary(environ: Mapping[str, str], summary: str) -> None: print(summary) summary_path = environ.get(GITHUB_STEP_SUMMARY, "").strip() if summary_path: - with open(summary_path, "a", encoding="utf-8") as summary_file: + with Path(summary_path).open("a", encoding="utf-8") as summary_file: summary_file.write(summary) diff --git a/policyengine_api/data/v2/privilege_requirements.py b/policyengine_api/data/v2/privilege_requirements.py index 154fd6404..f159f747f 100644 --- a/policyengine_api/data/v2/privilege_requirements.py +++ b/policyengine_api/data/v2/privilege_requirements.py @@ -101,15 +101,17 @@ def _annotation_nodes(module: ast.Module) -> set[int]: if node.returns is not None: annotation_roots.append(node.returns) arguments = node.args - for argument in [ - *arguments.posonlyargs, - *arguments.args, - *arguments.kwonlyargs, - *([arguments.vararg] if arguments.vararg else []), - *([arguments.kwarg] if arguments.kwarg else []), - ]: - if argument.annotation is not None: - annotation_roots.append(argument.annotation) + annotation_roots.extend( + argument.annotation + for argument in [ + *arguments.posonlyargs, + *arguments.args, + *arguments.kwonlyargs, + *([arguments.vararg] if arguments.vararg else []), + *([arguments.kwarg] if arguments.kwarg else []), + ] + if argument.annotation is not None + ) elif isinstance(node, ast.AnnAssign): annotation_roots.append(node.annotation) return {id(inner) for root in annotation_roots for inner in ast.walk(root)} diff --git a/policyengine_api/fastapi_routes/dependencies.py b/policyengine_api/fastapi_routes/dependencies.py index fb3455339..b3b84ec4d 100644 --- a/policyengine_api/fastapi_routes/dependencies.py +++ b/policyengine_api/fastapi_routes/dependencies.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from functools import lru_cache from importlib import metadata as importlib_metadata -from typing import TYPE_CHECKING, Protocol +from typing import TYPE_CHECKING, Annotated, Protocol from fastapi import Depends from fastapi.security import APIKeyHeader, HTTPAuthorizationCredentials, HTTPBearer @@ -791,8 +791,10 @@ def _default_v2_household_service_factory() -> V2HouseholdResourceService: def _default_require_user( - credentials: HTTPAuthorizationCredentials | None = Depends(_user_bearer_scheme), - unauthenticated_user: str | None = Depends(_unauthenticated_user_scheme), + credentials: Annotated[ + HTTPAuthorizationCredentials | None, Depends(_user_bearer_scheme) + ], + unauthenticated_user: Annotated[str | None, Depends(_unauthenticated_user_scheme)], ) -> AccountPrincipal: """Authenticate the request's account; fails closed without a credential.""" return authenticate_account_request( diff --git a/policyengine_api/fastapi_routes/v2/me/routes.py b/policyengine_api/fastapi_routes/v2/me/routes.py index 3c64d0659..4b756209a 100644 --- a/policyengine_api/fastapi_routes/v2/me/routes.py +++ b/policyengine_api/fastapi_routes/v2/me/routes.py @@ -156,7 +156,7 @@ def patch() -> MeDetailResponse: ), ) def delete_me( - query: CountryQueryDependency, + _query: CountryQueryDependency, principal: Annotated[AccountPrincipal, Depends(require_user)], ) -> Response | JSONResponse: def delete() -> Response: diff --git a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py index dfe11f152..2ed036387 100644 --- a/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py +++ b/policyengine_api/fastapi_routes/v2/metadata/parameter_routes.py @@ -114,7 +114,7 @@ def list_parameter_values( country_id: str, policyengine_version: str | None = None, parameter_id: UUID | None = None, - current: bool = False, + current: bool = False, # noqa: FBT001, FBT002 # FastAPI query parameter offset: Offset = 0, limit: Limit = 100, ) -> MetadataParameterValuePageResponse | JSONResponse: diff --git a/policyengine_api/libs/simulation_api.py b/policyengine_api/libs/simulation_api.py index a3aa22d15..94a274f8e 100644 --- a/policyengine_api/libs/simulation_api.py +++ b/policyengine_api/libs/simulation_api.py @@ -20,8 +20,8 @@ "ModalBudgetWindowBatchExecution", "ModalSimulationExecution", "SimulationAPIClient", - "SimulationEntrypointClient", "SimulationAPIModal", + "SimulationEntrypointClient", "resolve_simulation_api_url", "resolve_simulation_entrypoint_url", "simulation_api", diff --git a/policyengine_api/libs/simulation_api_modal.py b/policyengine_api/libs/simulation_api_modal.py index 2c472af46..72a8c0822 100644 --- a/policyengine_api/libs/simulation_api_modal.py +++ b/policyengine_api/libs/simulation_api_modal.py @@ -17,8 +17,8 @@ "ModalBudgetWindowBatchExecution", "ModalSimulationExecution", "SimulationAPIClient", - "SimulationEntrypointClient", "SimulationAPIModal", + "SimulationEntrypointClient", "resolve_simulation_api_url", "resolve_simulation_entrypoint_url", "simulation_api", diff --git a/policyengine_api/libs/simulation_entrypoint.py b/policyengine_api/libs/simulation_entrypoint.py index b7bba4b48..60702ace3 100644 --- a/policyengine_api/libs/simulation_entrypoint.py +++ b/policyengine_api/libs/simulation_entrypoint.py @@ -356,7 +356,7 @@ def run(self, payload: dict) -> ModalSimulationExecution: except httpx.RequestError as e: logger.log_struct( { - "message": f"Simulation entrypoint request error: {str(e)}", + "message": f"Simulation entrypoint request error: {e!s}", "observability_id": current_observability_id(), }, severity="ERROR", @@ -405,7 +405,7 @@ def run_budget_window_batch(self, payload: dict) -> ModalBudgetWindowBatchExecut except httpx.RequestError as e: logger.log_struct( { - "message": f"Simulation batch API request error: {str(e)}", + "message": f"Simulation batch API request error: {e!s}", "observability_id": current_observability_id(), }, severity="ERROR", @@ -545,7 +545,7 @@ def get_execution_by_id(self, job_id: str) -> ModalSimulationExecution: { "message": ( f"Simulation entrypoint request error polling job {job_id}: " - f"{str(e)}" + f"{e!s}" ), }, severity="ERROR", @@ -595,7 +595,7 @@ def get_budget_window_batch_by_id( { "message": ( "Simulation batch API request error polling job " - f"{batch_job_id}: {str(e)}" + f"{batch_job_id}: {e!s}" ), }, severity="ERROR", diff --git a/policyengine_api/readiness.py b/policyengine_api/readiness.py index f94255c82..d668c9f42 100644 --- a/policyengine_api/readiness.py +++ b/policyengine_api/readiness.py @@ -10,8 +10,9 @@ import threading -_lock = threading.Lock() -_ready = True +# Set while the service is warmed up. It starts set, because the default is ready. +_warmed_up = threading.Event() +_warmed_up.set() def validate_policy_runtime_configuration() -> None: @@ -52,23 +53,17 @@ def validate_policy_runtime_configuration() -> None: def mark_not_ready() -> None: """Report not-ready — call before running the startup warmup.""" - global _ready - with _lock: - _ready = False + _warmed_up.clear() def mark_ready() -> None: """Report ready — call once the startup warmup has completed.""" - global _ready - with _lock: - _ready = True + _warmed_up.set() def is_ready() -> bool: """Whether the service is warmed up and can serve a real request quickly.""" - with _lock: - warmed_up = _ready - if not warmed_up: + if not _warmed_up.is_set(): return False try: validate_policy_runtime_configuration() diff --git a/policyengine_api/routes/household_routes.py b/policyengine_api/routes/household_routes.py index cf0ac0b0d..4dfcaec65 100644 --- a/policyengine_api/routes/household_routes.py +++ b/policyengine_api/routes/household_routes.py @@ -177,9 +177,11 @@ def _spm_error_response(error: Exception) -> Response | None: def _calculation_response(calculation: HouseholdCalculationResult) -> dict: - result: dict[str, object] = dict( - status="ok", message=None, result=calculation.household - ) + result: dict[str, object] = { + "status": "ok", + "message": None, + "result": calculation.household, + } if calculation.warnings: result["warnings"] = list(calculation.warnings) if calculation.spm_provenance is not None: @@ -282,7 +284,7 @@ def wrapped( return decorator -def _calculation_cache_key(*args: object, **kwargs: object) -> str: +def _calculation_cache_key(*_args: object, **_kwargs: object) -> str: country_id = request.view_args["country_id"] return hash_object( { @@ -516,7 +518,7 @@ def _calculate() -> dict | Response: @validate_country @_validate_calculation_request(add_missing=False) @cache.cached(make_cache_key=_calculation_cache_key) -def get_calculate(country_id: str) -> dict | Response: +def get_calculate(country_id: str) -> dict | Response: # noqa: ARG001 # Flask passes the URL variable by keyword """Calculate a household without adding omitted yearly variables.""" return _calculate() @@ -525,6 +527,6 @@ def get_calculate(country_id: str) -> dict | Response: @validate_country @_validate_calculation_request(add_missing=True) @cache.cached(make_cache_key=_calculation_cache_key) -def get_calculate_full(country_id: str) -> dict | Response: +def get_calculate_full(country_id: str) -> dict | Response: # noqa: ARG001 # Flask passes the URL variable by keyword """Calculate a household after adding omitted yearly variables.""" return _calculate() diff --git a/policyengine_api/routes/policy_routes.py b/policyengine_api/routes/policy_routes.py index 72db93ca0..729c66a16 100644 --- a/policyengine_api/routes/policy_routes.py +++ b/policyengine_api/routes/policy_routes.py @@ -298,13 +298,13 @@ def set_policy(country_id: str) -> Response: if mirror_to_v2: _copy_policy_to_v2(country_id, policy_id, getattr(creation, "snapshot", None)) - response_body = dict( - status="ok", - message=message, - result=dict( - policy_id=policy_id, - ), - ) + response_body = { + "status": "ok", + "message": message, + "result": { + "policy_id": policy_id, + }, + } code = 200 if is_existing_policy else 201 return Response(json.dumps(response_body), status=code, mimetype="application/json") @@ -342,14 +342,14 @@ def get_policy_search(country_id: str) -> Response: 404, ) - policies = [dict(id=result.id, label=result.label) for result in results] + policies = [{"id": result.id, "label": result.label} for result in results] return Response( json.dumps( - dict( - status="ok", - message="Policies found", - result=policies, - ) + { + "status": "ok", + "message": "Policies found", + "result": policies, + } ), status=200, mimetype="application/json", @@ -433,14 +433,14 @@ def set_user_policy(country_id: str) -> Response: if not creation.created: return Response( json.dumps( - dict( - status="ok", - message=( + { + "status": "ok", + "message": ( f"The reform #{reform_id} / baseline #{baseline_id} pair " f"already exists for user {user_id}" ), - result=dict(id=user_policy.id), - ) + "result": {"id": user_policy.id}, + } ), status=200, mimetype="application/json", @@ -448,11 +448,11 @@ def set_user_policy(country_id: str) -> Response: return Response( json.dumps( - dict( - status="ok", - message="Record created successfully", - result=_serialize_user_policy(user_policy), - ) + { + "status": "ok", + "message": "Record created successfully", + "result": _serialize_user_policy(user_policy), + } ), status=201, mimetype="application/json", @@ -468,11 +468,11 @@ def get_user_policy(country_id: str, user_id: str) -> dict | Response: except ValueError: return _policy_configuration_unavailable() user_policies = user_policy_service.list_user_policies(country_id, user_id) - return dict( - status="ok", - message=None, - result=[_serialize_user_policy(row) for row in user_policies], - ) + return { + "status": "ok", + "message": None, + "result": [_serialize_user_policy(row) for row in user_policies], + } UPDATE_USER_POLICY_ALLOWED_FIELDS = USER_POLICY_MUTABLE_FIELDS @@ -550,11 +550,11 @@ def update_user_policy(country_id: str) -> Response: return Response( json.dumps( - dict( - status="ok", - message="Record updated successfully", - result=dict(id=user_policy_id), - ) + { + "status": "ok", + "message": "Record updated successfully", + "result": {"id": user_policy_id}, + } ), status=200, mimetype="application/json", diff --git a/policyengine_api/routes/report_output_routes.py b/policyengine_api/routes/report_output_routes.py index d1c588fed..98cefed86 100644 --- a/policyengine_api/routes/report_output_routes.py +++ b/policyengine_api/routes/report_output_routes.py @@ -105,11 +105,11 @@ def create_report_output(country_id: str) -> Response: ) status_code = 201 if creation.created else 200 - response_body = dict( - status="ok", - message=message, - result=result, - ) + response_body = { + "status": "ok", + "message": message, + "result": result, + } return Response( json.dumps(response_body), @@ -126,7 +126,7 @@ def create_report_output(country_id: str) -> Response: "Bad request creating report output for country %s: %s", country_id, e ) msg = f"Failed to create report output: {e}" - raise BadRequest(msg) + raise BadRequest(msg) from e except Exception: current_app.logger.exception( "Unexpected error creating report output for country %s", country_id @@ -155,11 +155,11 @@ def get_report_output(country_id: str, report_id: int) -> Response: raise NotFound(msg) result = _serialize_v1_report_output(view) - response_body = dict( - status="ok", - message=None, - result=result, - ) + response_body = { + "status": "ok", + "message": None, + "result": result, + } return Response( json.dumps(response_body), @@ -223,11 +223,11 @@ def update_report_output(country_id: str) -> Response: ) result = _serialize_v1_report_output(view) - response_body = dict( - status="ok", - message="Report output updated successfully", - result=result, - ) + response_body = { + "status": "ok", + "message": "Report output updated successfully", + "result": result, + } return Response( json.dumps(response_body), @@ -250,7 +250,7 @@ def update_report_output(country_id: str) -> Response: e, ) msg = f"Failed to update report output: {e}" - raise BadRequest(msg) + raise BadRequest(msg) from e except Exception: current_app.logger.exception( "Unexpected error updating report #%s for country %s", diff --git a/policyengine_api/routes/simulation_routes.py b/policyengine_api/routes/simulation_routes.py index d3d127d9e..0c74ab322 100644 --- a/policyengine_api/routes/simulation_routes.py +++ b/policyengine_api/routes/simulation_routes.py @@ -115,11 +115,11 @@ def create_simulation(country_id: str) -> Response: ) status_code = 201 if creation.created else 200 - response_body = dict( - status="ok", - message=message, - result=result, - ) + response_body = { + "status": "ok", + "message": message, + "result": result, + } return Response( json.dumps(response_body), @@ -136,7 +136,7 @@ def create_simulation(country_id: str) -> Response: "Bad request creating simulation for country %s: %s", country_id, e ) msg = f"Failed to create simulation: {e}" - raise BadRequest(msg) + raise BadRequest(msg) from e except Exception: current_app.logger.exception( "Unexpected error creating simulation for country %s", country_id @@ -168,11 +168,11 @@ def get_simulation(country_id: str, simulation_id: int) -> Response: msg = f"Simulation #{simulation_id} not found." raise NotFound(msg) - response_body = dict( - status="ok", - message=None, - result=result, - ) + response_body = { + "status": "ok", + "message": None, + "result": result, + } return Response( json.dumps(response_body), @@ -234,11 +234,11 @@ def update_simulation(country_id: str) -> Response: result = _serialize_v1_simulation(simulation) - response_body = dict( - status="ok", - message="Simulation updated successfully", - result=result, - ) + response_body = { + "status": "ok", + "message": "Simulation updated successfully", + "result": result, + } return Response( json.dumps(response_body), @@ -261,7 +261,7 @@ def update_simulation(country_id: str) -> Response: e, ) msg = f"Failed to update simulation: {e}" - raise BadRequest(msg) + raise BadRequest(msg) from e except Exception: current_app.logger.exception( "Unexpected error updating simulation #%s for country %s", diff --git a/policyengine_api/routes/user_profile_routes.py b/policyengine_api/routes/user_profile_routes.py index 3cc175b5b..59ff29f85 100644 --- a/policyengine_api/routes/user_profile_routes.py +++ b/policyengine_api/routes/user_profile_routes.py @@ -54,11 +54,11 @@ def set_user_profile(country_id: str) -> Response: ) result = _serialize_user_profile(profile, include_auth0_id=False) - response = dict( - status="ok", - message="Record created successfully" if created else "Record exists", - result=result, - ) + response = { + "status": "ok", + "message": "Record created successfully" if created else "Record exists", + "result": result, + } return Response( json.dumps(response), status=201 if created else 200, @@ -68,7 +68,7 @@ def set_user_profile(country_id: str) -> Response: @user_profile_bp.route("//user-profile", methods=["GET"]) @validate_country -def get_user_profile(country_id: str) -> Response: +def get_user_profile(country_id: str) -> Response: # noqa: ARG001 # Flask passes the URL variable by keyword """Return a user profile looked up by ``auth0_id`` or, failing that, ``user_id``. The Auth0 ID is included only when the caller looked the profile up by it. @@ -99,11 +99,11 @@ def get_user_profile(country_id: str) -> Response: msg = "No such user" raise NotFound(msg) - response_body = dict( - status="ok", - message=f"User #{readable_row['user_id']} found successfully", - result=readable_row, - ) + response_body = { + "status": "ok", + "message": f"User #{readable_row['user_id']} found successfully", + "result": readable_row, + } return Response( json.dumps(response_body), @@ -114,7 +114,7 @@ def get_user_profile(country_id: str) -> Response: @user_profile_bp.route("//user-profile", methods=["PUT"]) @validate_country -def update_user_profile(country_id: str) -> Response: +def update_user_profile(country_id: str) -> Response: # noqa: ARG001 # Flask passes the URL variable by keyword """Update the username, primary country or start date of profile ``user_id``. The Auth0 ID cannot be changed: an ``auth0_id`` in the payload is ignored. @@ -149,11 +149,11 @@ def update_user_profile(country_id: str) -> Response: msg = "No such user id" raise NotFound(msg) - response_body = dict( - status="ok", - message=f"User profile #{user_id} updated successfully", - result=dict(user_id=user_id), - ) + response_body = { + "status": "ok", + "message": f"User profile #{user_id} updated successfully", + "result": {"user_id": user_id}, + } return Response( json.dumps(response_body), diff --git a/policyengine_api/runtime_cache/core.py b/policyengine_api/runtime_cache/core.py index 5c1540f43..229c4ad7f 100644 --- a/policyengine_api/runtime_cache/core.py +++ b/policyengine_api/runtime_cache/core.py @@ -73,7 +73,7 @@ def delete(self, *keys: str) -> object: """Delete every given key and return how many existed.""" ... - def pipeline(self, transaction: bool = True) -> CachePipeline: + def pipeline(self, *, transaction: bool = True) -> CachePipeline: """Return a pipeline whose commands run together on ``execute``.""" ... diff --git a/policyengine_api/runtime_cache/fake.py b/policyengine_api/runtime_cache/fake.py index 7d09895aa..86ec86598 100644 --- a/policyengine_api/runtime_cache/fake.py +++ b/policyengine_api/runtime_cache/fake.py @@ -5,6 +5,8 @@ from threading import RLock from typing import TYPE_CHECKING, Self +from typing_extensions import override + if TYPE_CHECKING: from collections.abc import Callable @@ -167,7 +169,7 @@ def eval( return 0 return self.delete(key) - def pipeline(self, transaction: bool = True) -> InMemoryPipeline: + def pipeline(self, *, transaction: bool = True) -> InMemoryPipeline: """Return a pipeline that queues commands until ``execute``.""" return InMemoryPipeline(self, transaction=transaction) @@ -217,42 +219,51 @@ def __exit__(self, *_args: object) -> None: class DisabledCacheBackend(InMemoryCacheBackend): """No-storage backend for unselected unit-test/application cache mode.""" + @override def get(self, key: str) -> None: """Return None: every read is a miss.""" - def set(self, key: str, value: str, **kwargs: object) -> bool: + @override + def set(self, key: str, value: str, **_kwargs: object) -> bool: """Discard the value and report the write as not stored.""" return False - def delete(self, *keys: str) -> int: + @override + def delete(self, *_keys: str) -> int: """Return 0: no key ever exists.""" return 0 + @override def zadd(self, key: str, mapping: dict[str, float]) -> int: """Discard the members and report none added.""" return 0 + @override def zrevrange(self, key: str, start: int, end: int) -> list[str]: """Return an empty list: no sorted set ever exists.""" return [] - def zrem(self, key: str, *members: str) -> int: + @override + def zrem(self, key: str, *_members: str) -> int: """Return 0: there is nothing to remove.""" return 0 + @override def zremrangebyrank(self, key: str, start: int, end: int) -> int: """Return 0: there is nothing to remove.""" return 0 + @override def expire(self, key: str, seconds: int) -> bool: """Return False, as Redis does for a key that does not exist.""" return False + @override def eval( self, _script: str, numkeys: int, - *keys_and_args: str, + *_keys_and_args: str, ) -> int: """Raise RuntimeError, so claim operations fail closed when disabled.""" msg = "the shared runtime cache is disabled" diff --git a/policyengine_api/services/economy_comparison.py b/policyengine_api/services/economy_comparison.py index eccf99f22..2b917cd72 100644 --- a/policyengine_api/services/economy_comparison.py +++ b/policyengine_api/services/economy_comparison.py @@ -22,14 +22,14 @@ def budgetary_impact(baseline: dict, reform: dict) -> dict: state_tax_revenue_impact = reform["total_state_tax"] - baseline["total_state_tax"] benefit_spending_impact = reform["total_benefits"] - baseline["total_benefits"] budgetary_impact = tax_revenue_impact - benefit_spending_impact - return dict( - budgetary_impact=budgetary_impact, - tax_revenue_impact=tax_revenue_impact, - state_tax_revenue_impact=state_tax_revenue_impact, - benefit_spending_impact=benefit_spending_impact, - households=sum(baseline["household_weight"]), - baseline_net_income=baseline["total_net_income"], - ) + return { + "budgetary_impact": budgetary_impact, + "tax_revenue_impact": tax_revenue_impact, + "state_tax_revenue_impact": state_tax_revenue_impact, + "benefit_spending_impact": benefit_spending_impact, + "households": sum(baseline["household_weight"]), + "baseline_net_income": baseline["total_net_income"], + } def labor_supply_response(baseline: dict, reform: dict) -> dict: @@ -63,56 +63,58 @@ def labor_supply_response(baseline: dict, reform: dict) -> dict: substitution_lsr_hh = MicroSeries(substitution_lsr_hh, weights=household_weight) income_lsr_hh = MicroSeries(income_lsr_hh, weights=household_weight) - decile_avg = dict( - income=income_lsr_hh.groupby(decile).mean().to_dict(), - substitution=substitution_lsr_hh.groupby(decile).mean().to_dict(), - ) - decile_rel = dict( - income=( + decile_avg = { + "income": income_lsr_hh.groupby(decile).mean().to_dict(), + "substitution": substitution_lsr_hh.groupby(decile).mean().to_dict(), + } + decile_rel = { + "income": ( income_lsr_hh.groupby(decile).sum() / original_earnings.groupby(decile).sum() ).to_dict(), - substitution=( + "substitution": ( substitution_lsr_hh.groupby(decile).sum() / original_earnings.groupby(decile).sum() ).to_dict(), - ) + } - relative_lsr = dict( - income=(income_lsr_hh.sum() / original_earnings.sum()), - substitution=(substitution_lsr_hh.sum() / original_earnings.sum()), - ) + relative_lsr = { + "income": (income_lsr_hh.sum() / original_earnings.sum()), + "substitution": (substitution_lsr_hh.sum() / original_earnings.sum()), + } decile_rel["income"] = {int(k): v for k, v in decile_rel["income"].items() if k > 0} decile_rel["substitution"] = { int(k): v for k, v in decile_rel["substitution"].items() if k > 0 } - hours = dict( - baseline=baseline["weekly_hours"], - reform=reform["weekly_hours"], - change=reform["weekly_hours"] - baseline["weekly_hours"], - income_effect=reform["weekly_hours_income_effect"] + hours = { + "baseline": baseline["weekly_hours"], + "reform": reform["weekly_hours"], + "change": reform["weekly_hours"] - baseline["weekly_hours"], + "income_effect": reform["weekly_hours_income_effect"] - baseline["weekly_hours_income_effect"], - substitution_effect=reform["weekly_hours_substitution_effect"] + "substitution_effect": reform["weekly_hours_substitution_effect"] - baseline["weekly_hours_substitution_effect"], - ) + } - return dict( - substitution_lsr=substitution_lsr, - income_lsr=income_lsr, - relative_lsr=relative_lsr, - total_change=total_change, - revenue_change=revenue_change, - decile=dict( - average=decile_avg, - relative=decile_rel, - ), - hours=hours, - ) + return { + "substitution_lsr": substitution_lsr, + "income_lsr": income_lsr, + "relative_lsr": relative_lsr, + "total_change": total_change, + "revenue_change": revenue_change, + "decile": { + "average": decile_avg, + "relative": decile_rel, + }, + "hours": hours, + } -def detailed_budgetary_impact(baseline: dict, reform: dict, country_id: str) -> dict: +def detailed_budgetary_impact( + baseline: dict, reform: dict, country_id: str | None +) -> dict: """Return baseline, reform and difference for each UK programme's budget. Only the UK reports programme-level totals; other countries get an empty dict. @@ -121,11 +123,12 @@ def detailed_budgetary_impact(baseline: dict, reform: dict, country_id: str) -> if country_id == "uk": for program in baseline["programs"]: # baseline[programs][program] = total budgetary impact of program - result[program] = dict( - baseline=baseline["programs"][program], - reform=reform["programs"][program], - difference=reform["programs"][program] - baseline["programs"][program], - ) + result[program] = { + "baseline": baseline["programs"][program], + "reform": reform["programs"][program], + "difference": reform["programs"][program] + - baseline["programs"][program], + } return result @@ -164,10 +167,10 @@ def decile_impact(baseline: dict, reform: dict) -> dict: ) rel_decile_dict = rel_income_change_by_decile.to_dict() avg_decile_dict = avg_income_change_by_decile.to_dict() - return dict( - relative={int(k): v for k, v in rel_decile_dict.items()}, - average={int(k): v for k, v in avg_decile_dict.items()}, - ) + return { + "relative": {int(k): v for k, v in rel_decile_dict.items()}, + "average": {int(k): v for k, v in avg_decile_dict.items()}, + } def wealth_decile_impact(baseline: dict, reform: dict) -> dict: @@ -204,10 +207,10 @@ def wealth_decile_impact(baseline: dict, reform: dict) -> dict: ) rel_decile_dict = rel_income_change_by_decile.to_dict() avg_decile_dict = avg_income_change_by_decile.to_dict() - return dict( - relative={int(k): v for k, v in rel_decile_dict.items()}, - average={int(k): v for k, v in avg_decile_dict.items()}, - ) + return { + "relative": {int(k): v for k, v in rel_decile_dict.items()}, + "average": {int(k): v for k, v in avg_decile_dict.items()}, + } def inequality_impact(baseline: dict, reform: dict) -> dict: @@ -221,20 +224,20 @@ def inequality_impact(baseline: dict, reform: dict) -> dict: dict: The impact of the reform on inequality. """ - return dict( - gini=dict( - baseline=baseline["gini"], - reform=reform["gini"], - ), - top_10_pct_share=dict( - baseline=baseline["top_10_percent_share"], - reform=reform["top_10_percent_share"], - ), - top_1_pct_share=dict( - baseline=baseline["top_1_percent_share"], - reform=reform["top_1_percent_share"], - ), - ) + return { + "gini": { + "baseline": baseline["gini"], + "reform": reform["gini"], + }, + "top_10_pct_share": { + "baseline": baseline["top_10_percent_share"], + "reform": reform["top_10_percent_share"], + }, + "top_1_pct_share": { + "baseline": baseline["top_1_percent_share"], + "reform": reform["top_1_percent_share"], + }, + } def poverty_impact(baseline: dict, reform: dict) -> dict: @@ -262,48 +265,48 @@ def poverty_impact(baseline: dict, reform: dict) -> dict: ) age = MicroSeries(baseline["age"]) - poverty = dict( - child=dict( - baseline=float(baseline_poverty[age < 18].mean()), - reform=float(reform_poverty[age < 18].mean()), - ), - adult=dict( - baseline=float(baseline_poverty[(age >= 18) & (age < 65)].mean()), - reform=float(reform_poverty[(age >= 18) & (age < 65)].mean()), - ), - senior=dict( - baseline=float(baseline_poverty[age >= 65].mean()), - reform=float(reform_poverty[age >= 65].mean()), - ), - all=dict( - baseline=float(baseline_poverty.mean()), - reform=float(reform_poverty.mean()), - ), - ) + poverty = { + "child": { + "baseline": float(baseline_poverty[age < 18].mean()), + "reform": float(reform_poverty[age < 18].mean()), + }, + "adult": { + "baseline": float(baseline_poverty[(age >= 18) & (age < 65)].mean()), + "reform": float(reform_poverty[(age >= 18) & (age < 65)].mean()), + }, + "senior": { + "baseline": float(baseline_poverty[age >= 65].mean()), + "reform": float(reform_poverty[age >= 65].mean()), + }, + "all": { + "baseline": float(baseline_poverty.mean()), + "reform": float(reform_poverty.mean()), + }, + } - deep_poverty = dict( - child=dict( - baseline=float(baseline_deep_poverty[age < 18].mean()), - reform=float(reform_deep_poverty[age < 18].mean()), - ), - adult=dict( - baseline=float(baseline_deep_poverty[(age >= 18) & (age < 65)].mean()), - reform=float(reform_deep_poverty[(age >= 18) & (age < 65)].mean()), - ), - senior=dict( - baseline=float(baseline_deep_poverty[age >= 65].mean()), - reform=float(reform_deep_poverty[age >= 65].mean()), - ), - all=dict( - baseline=float(baseline_deep_poverty.mean()), - reform=float(reform_deep_poverty.mean()), - ), - ) + deep_poverty = { + "child": { + "baseline": float(baseline_deep_poverty[age < 18].mean()), + "reform": float(reform_deep_poverty[age < 18].mean()), + }, + "adult": { + "baseline": float(baseline_deep_poverty[(age >= 18) & (age < 65)].mean()), + "reform": float(reform_deep_poverty[(age >= 18) & (age < 65)].mean()), + }, + "senior": { + "baseline": float(baseline_deep_poverty[age >= 65].mean()), + "reform": float(reform_deep_poverty[age >= 65].mean()), + }, + "all": { + "baseline": float(baseline_deep_poverty.mean()), + "reform": float(reform_deep_poverty.mean()), + }, + } - return dict( - poverty=poverty, - deep_poverty=deep_poverty, - ) + return { + "poverty": poverty, + "deep_poverty": deep_poverty, + } def compute_income_change( @@ -333,7 +336,7 @@ def intra_decile_impact(baseline: dict, reform: dict) -> dict: people = MicroSeries( baseline["household_count_people"], weights=baseline_income.weights ) - decile = MicroSeries(baseline["household_income_decile"]).values + decile = MicroSeries(baseline["household_income_decile"]).values # noqa: PD011 # MicroSeries overrides .values, and .to_numpy() emits a different warning income_change = compute_income_change(baseline_income.values, reform_income.values) # Within each decile, calculate the percentage of people who: @@ -354,7 +357,7 @@ def intra_decile_impact(baseline: dict, reform: dict) -> dict: "Gain more than 5%", ] for lower, upper, label in zip( - income_change_bounds[:-1], income_change_bounds[1:], outcome_labels + income_change_bounds[:-1], income_change_bounds[1:], outcome_labels, strict=True ): outcome_groups[label] = [] for i in range(1, 11): @@ -374,7 +377,7 @@ def intra_decile_impact(baseline: dict, reform: dict) -> dict: outcome_groups[label].append(people_in_proportion) all_outcomes[label] = sum(outcome_groups[label]) / 10 - return dict(deciles=outcome_groups, all=all_outcomes) + return {"deciles": outcome_groups, "all": all_outcomes} def intra_wealth_decile_impact(baseline: dict, reform: dict) -> dict: @@ -392,7 +395,7 @@ def intra_wealth_decile_impact(baseline: dict, reform: dict) -> dict: people = MicroSeries( baseline["household_count_people"], weights=baseline_income.weights ) - decile = MicroSeries(baseline["household_wealth_decile"]).values + decile = MicroSeries(baseline["household_wealth_decile"]).values # noqa: PD011 # MicroSeries overrides .values, and .to_numpy() emits a different warning income_change = compute_income_change(baseline_income.values, reform_income.values) # Within each decile, calculate the percentage of people who: @@ -413,7 +416,7 @@ def intra_wealth_decile_impact(baseline: dict, reform: dict) -> dict: "Gain more than 5%", ] for lower, upper, label in zip( - income_change_bounds[:-1], income_change_bounds[1:], outcome_labels + income_change_bounds[:-1], income_change_bounds[1:], outcome_labels, strict=True ): outcome_groups[label] = [] for i in range(1, 11): @@ -433,7 +436,7 @@ def intra_wealth_decile_impact(baseline: dict, reform: dict) -> dict: outcome_groups[label].append(people_in_proportion) all_outcomes[label] = sum(outcome_groups[label]) / 10 - return dict(deciles=outcome_groups, all=all_outcomes) + return {"deciles": outcome_groups, "all": all_outcomes} def poverty_gender_breakdown(baseline: dict, reform: dict) -> dict: @@ -463,32 +466,32 @@ def poverty_gender_breakdown(baseline: dict, reform: dict) -> dict: ) is_male = MicroSeries(baseline["is_male"]) - poverty = dict( - male=dict( - baseline=float(baseline_poverty[is_male].mean()), - reform=float(reform_poverty[is_male].mean()), - ), - female=dict( - baseline=float(baseline_poverty[~is_male].mean()), - reform=float(reform_poverty[~is_male].mean()), - ), - ) + poverty = { + "male": { + "baseline": float(baseline_poverty[is_male].mean()), + "reform": float(reform_poverty[is_male].mean()), + }, + "female": { + "baseline": float(baseline_poverty[~is_male].mean()), + "reform": float(reform_poverty[~is_male].mean()), + }, + } - deep_poverty = dict( - male=dict( - baseline=float(baseline_deep_poverty[is_male].mean()), - reform=float(reform_deep_poverty[is_male].mean()), - ), - female=dict( - baseline=float(baseline_deep_poverty[~is_male].mean()), - reform=float(reform_deep_poverty[~is_male].mean()), - ), - ) + deep_poverty = { + "male": { + "baseline": float(baseline_deep_poverty[is_male].mean()), + "reform": float(reform_deep_poverty[is_male].mean()), + }, + "female": { + "baseline": float(baseline_deep_poverty[~is_male].mean()), + "reform": float(reform_deep_poverty[~is_male].mean()), + }, + } - return dict( - poverty=poverty, - deep_poverty=deep_poverty, - ) + return { + "poverty": poverty, + "deep_poverty": deep_poverty, + } def poverty_racial_breakdown(baseline: dict, reform: dict) -> dict: @@ -512,28 +515,28 @@ def poverty_racial_breakdown(baseline: dict, reform: dict) -> dict: ) race = MicroSeries(baseline["race"]) # Can be WHITE, BLACK, HISPANIC, or OTHER. - poverty = dict( - white=dict( - baseline=float(baseline_poverty[race == "WHITE"].mean()), - reform=float(reform_poverty[race == "WHITE"].mean()), - ), - black=dict( - baseline=float(baseline_poverty[race == "BLACK"].mean()), - reform=float(reform_poverty[race == "BLACK"].mean()), - ), - hispanic=dict( - baseline=float(baseline_poverty[race == "HISPANIC"].mean()), - reform=float(reform_poverty[race == "HISPANIC"].mean()), - ), - other=dict( - baseline=float(baseline_poverty[race == "OTHER"].mean()), - reform=float(reform_poverty[race == "OTHER"].mean()), - ), - ) + poverty = { + "white": { + "baseline": float(baseline_poverty[race == "WHITE"].mean()), + "reform": float(reform_poverty[race == "WHITE"].mean()), + }, + "black": { + "baseline": float(baseline_poverty[race == "BLACK"].mean()), + "reform": float(reform_poverty[race == "BLACK"].mean()), + }, + "hispanic": { + "baseline": float(baseline_poverty[race == "HISPANIC"].mean()), + "reform": float(reform_poverty[race == "HISPANIC"].mean()), + }, + "other": { + "baseline": float(baseline_poverty[race == "OTHER"].mean()), + "reform": float(reform_poverty[race == "OTHER"].mean()), + }, + } - return dict( - poverty=poverty, - ) + return { + "poverty": poverty, + } class UKConstituencyBreakdownByConstituency(BaseModel): @@ -577,7 +580,7 @@ class UKLocalAuthorityBreakdown(BaseModel): def uk_constituency_breakdown( - baseline: dict, reform: dict, country_id: str, region: str | None = None + baseline: dict, reform: dict, country_id: str | None, region: str | None = None ) -> UKConstituencyBreakdown | None: """Return each UK constituency's average and relative household income change. @@ -647,9 +650,12 @@ def uk_constituency_breakdown( code: str = constituency_names.iloc[i]["code"] # Filter to specific constituency if requested - if selected_constituency is not None: - if name != selected_constituency and code != selected_constituency: - continue + if ( + selected_constituency is not None + and name != selected_constituency + and code != selected_constituency + ): + continue # Filter to specific country if requested. Constituency codes # are prefixed with a single letter that identifies the @@ -710,7 +716,7 @@ def uk_constituency_breakdown( def uk_local_authority_breakdown( - baseline: dict, reform: dict, country_id: str, region: str | None = None + baseline: dict, reform: dict, country_id: str | None, region: str | None = None ) -> UKLocalAuthorityBreakdown | None: """Return each UK local authority's average and relative household income change. @@ -776,9 +782,8 @@ def uk_local_authority_breakdown( code: str = local_authority_names.iloc[i]["code"] # Filter to specific local authority if requested - if selected_la is not None: - if name != selected_la and code != selected_la: - continue + if selected_la is not None and name != selected_la and code != selected_la: + continue # Filter to specific country if requested if selected_country is not None: @@ -837,7 +842,7 @@ def uk_local_authority_breakdown( def compare_economic_outputs( baseline: dict, reform: dict, - country_id: str = None, + country_id: str | None = None, region: str | None = None, ) -> dict: """Compare the economic outputs of two economies. @@ -888,30 +893,30 @@ def compare_economic_outputs( wealth_decile_impact_data = {} intra_wealth_decile_impact_data = {} - return dict( - budget=budgetary_impact_data, - detailed_budget=detailed_budgetary_impact_data, - decile=decile_impact_data, - inequality=inequality_impact_data, - poverty=poverty_impact_data, - poverty_by_gender=poverty_by_gender_data, - poverty_by_race=poverty_by_race_data, - intra_decile=intra_decile_impact_data, - wealth_decile=wealth_decile_impact_data, - intra_wealth_decile=intra_wealth_decile_impact_data, - labor_supply_response=labor_supply_response_data, - constituency_impact=constituency_impact_data, - local_authority_impact=local_authority_impact_data, - ) + return { + "budget": budgetary_impact_data, + "detailed_budget": detailed_budgetary_impact_data, + "decile": decile_impact_data, + "inequality": inequality_impact_data, + "poverty": poverty_impact_data, + "poverty_by_gender": poverty_by_gender_data, + "poverty_by_race": poverty_by_race_data, + "intra_decile": intra_decile_impact_data, + "wealth_decile": wealth_decile_impact_data, + "intra_wealth_decile": intra_wealth_decile_impact_data, + "labor_supply_response": labor_supply_response_data, + "constituency_impact": constituency_impact_data, + "local_authority_impact": local_authority_impact_data, + } if baseline.get("type") == "cliff": - return dict( - baseline=dict( - cliff_gap=baseline["cliff_gap"], - cliff_share=baseline["cliff_share"], - ), - reform=dict( - cliff_gap=reform["cliff_gap"], - cliff_share=reform["cliff_share"], - ), - ) + return { + "baseline": { + "cliff_gap": baseline["cliff_gap"], + "cliff_share": baseline["cliff_share"], + }, + "reform": { + "cliff_gap": reform["cliff_gap"], + "cliff_share": reform["cliff_share"], + }, + } return None diff --git a/policyengine_api/services/economy_service.py b/policyengine_api/services/economy_service.py index a67fc4890..3894262d4 100644 --- a/policyengine_api/services/economy_service.py +++ b/policyengine_api/services/economy_service.py @@ -1557,6 +1557,7 @@ def _setup_sim_options( region: str, time_period: str, scope: Literal["macro", "household"] = "macro", + *, include_cliffs: bool = False, model_version: str | None = None, policyengine_version: str | None = None, @@ -1677,18 +1678,26 @@ def _with_policyengine_bundle( "dataset": setup_options.dataset, } if isinstance(result.get("policyengine_bundle"), dict): - for key, value in result["policyengine_bundle"].items(): - if value is not None: - bundle[key] = value + bundle.update( + { + key: value + for key, value in result["policyengine_bundle"].items() + if value is not None + } + ) execution_bundle = ( getattr(execution, "policyengine_bundle", None) if execution is not None else None ) if isinstance(execution_bundle, dict): - for key, value in execution_bundle.items(): - if value is not None: - bundle[key] = value + bundle.update( + { + key: value + for key, value in execution_bundle.items() + if value is not None + } + ) response = { **result, "policyengine_bundle": bundle, @@ -1819,7 +1828,7 @@ def _set_reform_impact_computing( except Exception as e: logger.log_struct( { - "message": f"Error inserting computing record: {str(e)}", + "message": f"Error inserting computing record: {e!s}", **setup_options.model_dump(), } ) @@ -1850,9 +1859,7 @@ def _set_reform_impact_complete( except Exception as e: logger.log_struct( { - "message": ( - f"Error setting completed reform impact record: {str(e)}" - ), + "message": (f"Error setting completed reform impact record: {e!s}"), **setup_options.model_dump(), } ) @@ -1885,7 +1892,7 @@ def _set_reform_impact_error( except Exception as e: logger.log_struct( { - "message": f"Error setting error reform impact record: {str(e)}", + "message": f"Error setting error reform impact record: {e!s}", **setup_options.model_dump(), } ) diff --git a/policyengine_api/services/reform_impacts_service.py b/policyengine_api/services/reform_impacts_service.py index 5ac0ba95c..813f2a8d9 100644 --- a/policyengine_api/services/reform_impacts_service.py +++ b/policyengine_api/services/reform_impacts_service.py @@ -1,5 +1,6 @@ """Recoverable reform-impact cache service backed by shared Redis.""" +import contextlib import datetime from typing import Any @@ -158,7 +159,7 @@ def release_reform_impact_start( observability_id: str, ) -> None: """Best-effort release; an unavailable cache safely falls back to expiry.""" - try: + with contextlib.suppress(CacheCoordinationError): self._cache.release_start( country_id=country_id, reform_policy_id=policy_id, @@ -172,8 +173,6 @@ def release_reform_impact_start( claim_token=claim_token, observability_id=observability_id, ) - except CacheCoordinationError: - pass def set_reform_impact( self, diff --git a/policyengine_api/services/v2/associations/validators.py b/policyengine_api/services/v2/associations/validators.py index 4d9075eb9..a5cff660e 100644 --- a/policyengine_api/services/v2/associations/validators.py +++ b/policyengine_api/services/v2/associations/validators.py @@ -44,7 +44,7 @@ def require_legacy_reference_checker( return legacy_references -def require_not_mirror_owned(mirror_owned: bool, *, association_id: UUID) -> None: +def require_not_mirror_owned(*, mirror_owned: bool, association_id: UUID) -> None: """Raise MirrorOwnedAssociationError if the v1 saved-policy mirror owns the row.""" if mirror_owned: msg = ( diff --git a/policyengine_api/services/v2/comparison_runs/__init__.py b/policyengine_api/services/v2/comparison_runs/__init__.py index a0e8f6084..52d7ddf5b 100644 --- a/policyengine_api/services/v2/comparison_runs/__init__.py +++ b/policyengine_api/services/v2/comparison_runs/__init__.py @@ -11,10 +11,10 @@ ) __all__ = [ - "ComparisonRunAggregationStatus", - "ComparisonRunLifecycleStatus", "ComparisonReportPersistenceResult", "ComparisonReportRecord", + "ComparisonRunAggregationStatus", + "ComparisonRunLifecycleStatus", "ComparisonSimulationPersistenceResult", "ComparisonSimulationRecord", "ResultComparisonStatus", diff --git a/policyengine_api/services/v2/user_policies/services.py b/policyengine_api/services/v2/user_policies/services.py index e79362dc8..aab030c43 100644 --- a/policyengine_api/services/v2/user_policies/services.py +++ b/policyengine_api/services/v2/user_policies/services.py @@ -558,7 +558,9 @@ def patch_user_policy( association_id=association_id, ) require_not_mirror_owned( - read_user_policy_mirror_exists(session, association_id=association.id), + mirror_owned=read_user_policy_mirror_exists( + session, association_id=association.id + ), association_id=association.id, ) repointed_policy_id = ( @@ -603,7 +605,9 @@ def delete_user_policy( association_id=association_id, ) require_not_mirror_owned( - read_user_policy_mirror_exists(session, association_id=association.id), + mirror_owned=read_user_policy_mirror_exists( + session, association_id=association.id + ), association_id=association.id, ) delete_user_policy(session, association) diff --git a/policyengine_api/services/v2/user_reports/services.py b/policyengine_api/services/v2/user_reports/services.py index f3e218bb8..0fae168de 100644 --- a/policyengine_api/services/v2/user_reports/services.py +++ b/policyengine_api/services/v2/user_reports/services.py @@ -109,7 +109,7 @@ def validate_linked_user_policy( if user_policy_id is None: return require_owned_user_policy( - read_owned_user_policy_exists( + owned=read_owned_user_policy_exists( session, user_id=user_id, country_id=country_id, diff --git a/policyengine_api/services/v2/user_reports/validators.py b/policyengine_api/services/v2/user_reports/validators.py index bea15ad78..7d670ed35 100644 --- a/policyengine_api/services/v2/user_reports/validators.py +++ b/policyengine_api/services/v2/user_reports/validators.py @@ -31,7 +31,7 @@ def require_user_report( return association -def require_owned_user_policy(owned: bool, *, user_policy_id: UUID) -> None: +def require_owned_user_policy(*, owned: bool, user_policy_id: UUID) -> None: """Raise AssociationLinkNotFoundError unless a linked saved policy is ``owned``. ``owned`` says whether the report's owner has that saved policy in the diff --git a/policyengine_api/spm.py b/policyengine_api/spm.py index 932149b93..2f87ba09c 100644 --- a/policyengine_api/spm.py +++ b/policyengine_api/spm.py @@ -444,7 +444,7 @@ def resolved_spm_settings(settings: object) -> dict | None: config = SPMSelection.model_validate(settings) except ValidationError: return None - if not REQUIRED_RESOLVED_SPM_FIELDS <= config.model_fields_set: + if not config.model_fields_set >= REQUIRED_RESOLVED_SPM_FIELDS: return None return {name: getattr(config, name) for name in SPMSelection.model_fields} @@ -479,7 +479,9 @@ def _installed_country_implements_spm(country_id: str) -> bool: COUNTRY_PACKAGE_NAMES, ) - package_name = dict(zip(COUNTRIES, COUNTRY_PACKAGE_NAMES)).get(country_id) + package_name = dict(zip(COUNTRIES, COUNTRY_PACKAGE_NAMES, strict=True)).get( + country_id + ) if package_name is None: return False try: diff --git a/policyengine_api/utils/cache_utils.py b/policyengine_api/utils/cache_utils.py index c5f8bf516..927f0d93b 100644 --- a/policyengine_api/utils/cache_utils.py +++ b/policyengine_api/utils/cache_utils.py @@ -8,8 +8,7 @@ import flask -def make_cache_key(*args: object, **kwargs: object) -> str: - # pylint: disable=unused-argument +def make_cache_key(*_args: object, **_kwargs: object) -> str: """Hash the current request's full path and JSON or form body into a cache key. The arguments are ignored. Keep it fast: adding overhead for a minor extra diff --git a/policyengine_api/utils/payload_validators/validate_household_payload.py b/policyengine_api/utils/payload_validators/validate_household_payload.py index 5eb1cbca9..f7c495e6d 100644 --- a/policyengine_api/utils/payload_validators/validate_household_payload.py +++ b/policyengine_api/utils/payload_validators/validate_household_payload.py @@ -28,9 +28,12 @@ def validate_household_payload( return False, f"Missing required keys: {missing_keys}" # Check that label is either string or None, if present - if "label" in payload: - if payload["label"] is not None and not isinstance(payload["label"], str): - return False, "Label must be a string or None" + if ( + "label" in payload + and payload["label"] is not None + and not isinstance(payload["label"], str) + ): + return False, "Label must be a string or None" # Check that data is a dictionary if not isinstance(payload["data"], dict): diff --git a/policyengine_api/utils/payload_validators/validate_set_policy_payload.py b/policyengine_api/utils/payload_validators/validate_set_policy_payload.py index 9c9d2f098..5e3bef36d 100644 --- a/policyengine_api/utils/payload_validators/validate_set_policy_payload.py +++ b/policyengine_api/utils/payload_validators/validate_set_policy_payload.py @@ -13,9 +13,12 @@ def validate_set_policy_payload(payload: dict) -> tuple[bool, str | None]: return False, f"Missing required keys: {missing_keys}" # Check that label is either string or None - if "label" in payload: - if payload["label"] is not None and not isinstance(payload["label"], str): - return False, "Label must be a string or None" + if ( + "label" in payload + and payload["label"] is not None + and not isinstance(payload["label"], str) + ): + return False, "Label must be a string or None" # Check that data is a dictionary if not isinstance(payload["data"], dict): diff --git a/policyengine_api/utils/singleton.py b/policyengine_api/utils/singleton.py index bc801566f..47217101a 100644 --- a/policyengine_api/utils/singleton.py +++ b/policyengine_api/utils/singleton.py @@ -1,10 +1,12 @@ """Metaclass for classes that have a single shared instance per process.""" +from typing import ClassVar + class Singleton(type): """Metaclass whose classes build one instance and return it on every later call.""" - _instances = {} + _instances: ClassVar[dict[type, object]] = {} def __call__(cls, *args: object, **kwargs: object) -> object: """Return the shared instance, creating it on the first call. diff --git a/policyengine_api/worker_spm_release.py b/policyengine_api/worker_spm_release.py index 9f050f032..d73f104f3 100644 --- a/policyengine_api/worker_spm_release.py +++ b/policyengine_api/worker_spm_release.py @@ -6,8 +6,10 @@ import sys from importlib.metadata import version as distribution_version +from typing_extensions import override + from policyengine_api import constants -from policyengine_api.worker_spm import validate_worker_spm +from policyengine_api.worker_spm import SPMCapabilitySource, validate_worker_spm REQUIRED_BUNDLE_PACKAGES = ( "policyengine", @@ -18,13 +20,14 @@ ) -class RegistryCapability: +class RegistryCapability(SPMCapabilitySource): """Expose one registry document through the request validator interface.""" def __init__(self, registry: dict) -> None: """Wrap a parsed worker registry document.""" self.registry = registry + @override def get_spm_capability( self, country: str, @@ -80,7 +83,7 @@ def validate_installed_worker(registry: dict, expected_bundle: str) -> dict | No if versions["policyengine"] != expected_bundle: msg = "The bundle manifest differs from the release requirement" raise ValueError(msg) - if constants.POLICYENGINE_VERSION != expected_bundle: + if expected_bundle != constants.POLICYENGINE_VERSION: msg = "The API runtime differs from the release requirement" raise ValueError(msg) diff --git a/pyproject.toml b/pyproject.toml index 5219f6d99..b565b5e24 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,6 +60,7 @@ dependencies = [ "sqlmodel>=0.0.39,<0.1", "psycopg[binary]>=3.3,<4", "pyjwt[crypto]>=2.10,<3", + "typing-extensions>=4.4,<5", ] [project.optional-dependencies] @@ -117,6 +118,8 @@ select = [ "DTZ", "ERA", "EXE", "FIX", "G", "LOG", "S", "T20", "TD", # Group 7: structure and imports. "A", "E", "FAST", "INP", "N", "PLC", "SLF", "TID253", + # Group 8a: arguments, idioms and pytest style. + "ARG", "ASYNC", "B", "C4", "FBT", "PD", "PERF", "PGH", "PLW", "PT", "PTH", "RUF", "SIM", ] # Mutually exclusive pydocstyle pairs: keep D211 (no blank line before a class # docstring) and D212 (summary on the first line). @@ -136,12 +139,17 @@ exclude = ["migrations/*/versions/*.py"] # live in directories that aren't packages, import inside a test (often after # patching the environment), and use fake credentials, naive datetimes, # subprocess calls to repository scripts, SQL built from strings and prints, -# may import the lazily loaded packages at module level, and may name a fake's -# methods after the interface it replaces. Every other rule applies to tests. +# may import the lazily loaded packages at module level, may name a fake's +# methods after the interface it replaces, and may leave a stand-in's +# arguments unused. Every other rule applies to tests. "**/tests/**" = [ - "ANN", "D1", "DTZ001", "DTZ011", "INP001", "N802", "PLC0415", "PLR2004", - "S101", "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", "TID253", + "ANN", "ARG001", "ARG002", "ARG005", "D1", "DTZ001", "DTZ011", "INP001", + "N802", "PLC0415", "PLR2004", "S101", "S105", "S106", "S603", "S607", + "S608", "SLF001", "T201", "TID253", ] +# The shared rules tool needs only the standard library, so its tests use +# unittest on purpose; pytest-style rules don't apply to them. +"scripts/rules/tests/**" = ["PT"] # CI/CD scripts print their results, fetch fixed URLs, may have hyphenated # file names and may import the lazily loaded packages at module level. ".github/**" = ["N999", "S310", "T201", "TID253"] diff --git a/rules/registry.toml b/rules/registry.toml index a2ad1fd66..8cdec5f83 100644 --- a/rules/registry.toml +++ b/rules/registry.toml @@ -773,7 +773,7 @@ autofix = true [rule.enforcer] kind = "ruff" mode = "lint" -codes = ["A", "ANN", "BLE", "D", "DTZ", "E", "E4", "E7", "E9", "EM", "ERA", "EXE", "F", "FAST", "FIX", "FLY", "FURB", "G", "I", "INP", "LOG", "N", "PIE", "PLC", "PYI", "RET", "RSE", "S", "SLF", "T20", "TC", "TD", "TID253", "TRY", "UP", "W"] +codes = ["A", "ANN", "ARG", "ASYNC", "B", "BLE", "C4", "D", "DTZ", "E", "E4", "E7", "E9", "EM", "ERA", "EXE", "F", "FAST", "FBT", "FIX", "FLY", "FURB", "G", "I", "INP", "LOG", "N", "PD", "PERF", "PGH", "PIE", "PLC", "PLW", "PT", "PTH", "PYI", "RET", "RSE", "RUF", "S", "SIM", "SLF", "T20", "TC", "TD", "TID253", "TRY", "UP", "W"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -817,7 +817,7 @@ kind = "none" [[rule]] id = "api.quality.tests-ruff-exemptions" category = "quality" -statement = "Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415). Their fakes may name methods after the interface they replace, such as a gRPC stub's Export (N802). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), and may import the lazily loaded packages at module level (TID253). Every other ruff rule applies to tests." +statement = "Test files may use pytest's bare assert, need no docstrings, annotations or named constants for magic numbers, and may reach into private members, live outside packages and import inside a test (S101, D1, ANN, PLR2004, SLF001, INP001, PLC0415). Their fakes may name methods after the interface they replace, such as a gRPC stub's Export (N802). They may also use fake credentials, naive datetimes, subprocess calls to repository scripts, SQL built from strings and prints (S105, S106, DTZ001, DTZ011, S603, S607, S608, T201), may import the lazily loaded packages at module level (TID253), and may leave a stand-in's arguments unused (ARG001, ARG002, ARG005). Every other ruff rule applies to tests." rationale = "docs/engineering/skills/rules-registry.md#file-level-exceptions" scope = ["tests/**/*.py", "scripts/rules/tests/**/*.py"] tier = "mechanical" @@ -827,7 +827,7 @@ autofix = false kind = "ruff" mode = "per-file-ignore" pattern = "**/tests/**" -codes = ["ANN", "D1", "DTZ001", "DTZ011", "INP001", "N802", "PLC0415", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", "TID253"] +codes = ["ANN", "ARG001", "ARG002", "ARG005", "D1", "DTZ001", "DTZ011", "INP001", "N802", "PLC0415", "PLR2004", "S101", "S105", "S106", "S603", "S607", "S608", "SLF001", "T201", "TID253"] [[rule.ci]] workflow = ".github/workflows/pr.yml" job = "lint" @@ -897,6 +897,25 @@ workflow = ".github/workflows/pr.yml" job = "lint" invocation = "ruff check ." + +[[rule]] +id = "api.quality.unittest-ruff-exemptions" +category = "quality" +statement = "The shared rules tool's tests use unittest, because the tool needs only the standard library, so ruff's pytest-style rules (PT) don't apply to them." +rationale = "docs/engineering/skills/rules-registry.md#baselines" +scope = ["scripts/rules/tests/**/*.py"] +tier = "mechanical" +status = "enforced" +autofix = false +[rule.enforcer] +kind = "ruff" +mode = "per-file-ignore" +pattern = "scripts/rules/tests/**" +codes = ["PT"] +[[rule.ci]] +workflow = ".github/workflows/pr.yml" +job = "lint" +invocation = "ruff check ." [[rule]] id = "api.quality.mypy-v2" category = "quality" diff --git a/scripts/compare_migration_baseline.py b/scripts/compare_migration_baseline.py index d3f320526..6451e95f9 100644 --- a/scripts/compare_migration_baseline.py +++ b/scripts/compare_migration_baseline.py @@ -136,8 +136,10 @@ def compare( f"(≤ {p95_limit:.2f}ms)" ) - for name in sorted(set(candidate_stats) - set(baseline_stats)): - lines.append(f"NOTE {name}: not in baseline; not gated") + lines.extend( + f"NOTE {name}: not in baseline; not gated" + for name in sorted(set(candidate_stats) - set(baseline_stats)) + ) return passed, lines diff --git a/scripts/guards/migration_contracts.py b/scripts/guards/migration_contracts.py index 3018357a5..95c65defa 100644 --- a/scripts/guards/migration_contracts.py +++ b/scripts/guards/migration_contracts.py @@ -23,16 +23,13 @@ def _check_unique_values( *, label: str, ) -> list[str]: - violations = [] seen: set[str] = set() duplicates: set[str] = set() for value in values: if value in seen: duplicates.add(value) seen.add(value) - for value in sorted(duplicates): - violations.append(f"duplicate {label}: {value!r}") - return violations + return [f"duplicate {label}: {value!r}" for value in sorted(duplicates)] def _check_route_groups(payload: dict[str, Any]) -> list[str]: diff --git a/scripts/measure_cloud_run_runtime.py b/scripts/measure_cloud_run_runtime.py index 80c1eff61..3914a18f7 100644 --- a/scripts/measure_cloud_run_runtime.py +++ b/scripts/measure_cloud_run_runtime.py @@ -151,8 +151,7 @@ def _summarize_endpoint(records: list[dict], duration_seconds: float) -> dict: } -async def _run(args: argparse.Namespace) -> dict: - base_payload = json.loads(Path(args.calculate_payload).read_text()) +async def _run(args: argparse.Namespace, base_payload: dict) -> dict: base_payload["household"]["axes"][0][0]["count"] = args.axes_count cache_bust = args.cache_bust == "always" run_token = uuid.uuid4().hex[:8] @@ -262,7 +261,8 @@ def main() -> int: ) args = parser.parse_args() - summary = asyncio.run(_run(args)) + base_payload = json.loads(Path(args.calculate_payload).read_text()) + summary = asyncio.run(_run(args, base_payload)) json.dump(summary, sys.stdout, indent=2) sys.stdout.write("\n") return 0 diff --git a/scripts/rules/adapters.py b/scripts/rules/adapters.py index a16c5b40c..81890218d 100644 --- a/scripts/rules/adapters.py +++ b/scripts/rules/adapters.py @@ -30,7 +30,7 @@ def check_adapters(agents: Agents, root: Path) -> list[Violation]: continue text = path.read_text() - def v(message: str, fix: str) -> None: + def v(message: str, fix: str, adapter: str = adapter) -> None: violations.append(Violation(None, "adapters", f"{adapter} {message}", fix)) if instructions not in text: diff --git a/scripts/rules/baseline.py b/scripts/rules/baseline.py index 2af9b1ac5..62fa76c6f 100644 --- a/scripts/rules/baseline.py +++ b/scripts/rules/baseline.py @@ -33,6 +33,7 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: cwd=root, capture_output=True, text=True, + check=False, ) if verify.returncode != 0: raise BaseUnavailableError(ref) @@ -41,6 +42,7 @@ def load_base_registry(root: Path, ref: str) -> Registry | None: cwd=root, capture_output=True, text=True, + check=False, ) if shown.returncode != 0: return None @@ -85,6 +87,8 @@ def check_baselines( "explicit and reviewable", ) ) - for rule_id in sorted(current_ids - {rule.id for rule in base.rules}): - notes.append(f"rule {rule_id} is new") + notes.extend( + f"rule {rule_id} is new" + for rule_id in sorted(current_ids - {rule.id for rule in base.rules}) + ) return violations, notes diff --git a/scripts/rules/ci_scripts.py b/scripts/rules/ci_scripts.py index 77fe38324..077c0f84d 100644 --- a/scripts/rules/ci_scripts.py +++ b/scripts/rules/ci_scripts.py @@ -101,6 +101,7 @@ def shellcheck_failures(config: CiScripts, root: Path) -> set[str] | None: cwd=root, capture_output=True, text=True, + check=False, ) return {line.split(":", 1)[0] for line in result.stdout.splitlines() if ":" in line} @@ -177,24 +178,25 @@ def compare(registry: Registry, findings: Findings, root: Path) -> list[Violatio "fix what `shellcheck