From 820840d9d46a7683eeba4970b3067ae9049a7c01 Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Mon, 2 Feb 2026 15:06:28 +0000 Subject: [PATCH 01/62] first query for flow sheets --- .gitignore | 1 + sql_scripts/flow_sheet_values.sql | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 sql_scripts/flow_sheet_values.sql diff --git a/.gitignore b/.gitignore index be3f5ce..5fac6cd 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ wheels/ # IDEs .idea/ +.vscode/ # settings files (should not be in the source tree anyway, but just in case) *.env diff --git a/sql_scripts/flow_sheet_values.sql b/sql_scripts/flow_sheet_values.sql new file mode 100644 index 0000000..77d6cc1 --- /dev/null +++ b/sql_scripts/flow_sheet_values.sql @@ -0,0 +1,13 @@ +select observation_datetime as datetime, +value_as_real, +unit, +value_as_text, +comment, +vo.visit_observation_type_id , +(select display_name +from star.visit_observation_type as vt +where vt.visit_observation_type_id = vo.visit_observation_type_id ) as observation +from star.visit_observation as vo +left join star.visit_observation_type as vt +on vo.visit_observation_type_id = vt.visit_observation_type_id +where hospital_visit_id = 'csn' \ No newline at end of file From fb3d6da7edf40527fdbb5df5a8abf161614e1768 Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Wed, 4 Feb 2026 10:30:40 +0000 Subject: [PATCH 02/62] building up scripts --- sql_scripts/flow_sheet_values.sql | 2 +- sql_scripts/lab_results.sql | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 sql_scripts/lab_results.sql diff --git a/sql_scripts/flow_sheet_values.sql b/sql_scripts/flow_sheet_values.sql index 77d6cc1..bbfdfb9 100644 --- a/sql_scripts/flow_sheet_values.sql +++ b/sql_scripts/flow_sheet_values.sql @@ -1,7 +1,7 @@ select observation_datetime as datetime, value_as_real, unit, -value_as_text, +value_as_text,£. comment, vo.visit_observation_type_id , (select display_name diff --git a/sql_scripts/lab_results.sql b/sql_scripts/lab_results.sql new file mode 100644 index 0000000..9160ba6 --- /dev/null +++ b/sql_scripts/lab_results.sql @@ -0,0 +1,14 @@ +select +result_last_modified_datetime as date, +value_as_real as value, +units, +abnormal_flag, +comment + + +from star.lab_result as r +join star.lab_order as o +on r.lab_order_id = o.lab_order_id +where o.hospital_visit_id = 'xxx' +and r.result_status like 'FINAL' +and r.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') From 8476351baada271db1b38a1545ce57096dcd5e90 Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Thu, 19 Feb 2026 14:54:35 +0000 Subject: [PATCH 03/62] adding more files --- sql_scripts/flow_sheet_values.sql | 22 ++++++++++++++++------ sql_scripts/get_hospital_visit_id.sql | 2 ++ sql_scripts/lab_results.sql | 12 +++++++++++- sql_scripts/lab_test_names.sql | 5 +++++ 4 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 sql_scripts/get_hospital_visit_id.sql create mode 100644 sql_scripts/lab_test_names.sql diff --git a/sql_scripts/flow_sheet_values.sql b/sql_scripts/flow_sheet_values.sql index bbfdfb9..2a6c9eb 100644 --- a/sql_scripts/flow_sheet_values.sql +++ b/sql_scripts/flow_sheet_values.sql @@ -1,13 +1,23 @@ +--get the flow sheet values for the particular visit +-- the flow sheet numbers are recorded as id_in_application in the visit_observation_type table +-- Temperature 6 +-- Noradrenalin 3040102622 +-- Metaraminol 12946 +-- Secretion amount 451120 +-- Sputum amount 302600 + + select observation_datetime as datetime, +(select display_name +from star.visit_observation_type as vt +where vt.visit_observation_type_id = vo.visit_observation_type_id ) as observation, value_as_real, unit, -value_as_text,£. +value_as_text, comment, -vo.visit_observation_type_id , -(select display_name -from star.visit_observation_type as vt -where vt.visit_observation_type_id = vo.visit_observation_type_id ) as observation +vo.visit_observation_type_id from star.visit_observation as vo left join star.visit_observation_type as vt on vo.visit_observation_type_id = vt.visit_observation_type_id -where hospital_visit_id = 'csn' \ No newline at end of file +where vt.id_in_application in ('6', '12946', '302600', '451120', '3040102622') +and hospital_visit_id = 'xx' \ No newline at end of file diff --git a/sql_scripts/get_hospital_visit_id.sql b/sql_scripts/get_hospital_visit_id.sql new file mode 100644 index 0000000..5a60221 --- /dev/null +++ b/sql_scripts/get_hospital_visit_id.sql @@ -0,0 +1,2 @@ +select hospital_visit_id from star.hospital_visit as hv +where hv.encounter = 'csn' \ No newline at end of file diff --git a/sql_scripts/lab_results.sql b/sql_scripts/lab_results.sql index 9160ba6..9309722 100644 --- a/sql_scripts/lab_results.sql +++ b/sql_scripts/lab_results.sql @@ -1,5 +1,14 @@ +-- This selects the values are units of lab tests +-- 1011 CRP +-- 722790196 CRP +-- 390793054 WCC +-- 390793057 WCC +-- 390793060 WCC + select result_last_modified_datetime as date, +(select name from star.lab_test_definition as ltd +where ltd.lab_test_definition_id = r.lab_test_definition_id) as name, value_as_real as value, units, abnormal_flag, @@ -9,6 +18,7 @@ comment from star.lab_result as r join star.lab_order as o on r.lab_order_id = o.lab_order_id -where o.hospital_visit_id = 'xxx' +where o.hospital_visit_id = 'xx' and r.result_status like 'FINAL' and r.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') + diff --git a/sql_scripts/lab_test_names.sql b/sql_scripts/lab_test_names.sql new file mode 100644 index 0000000..85ce715 --- /dev/null +++ b/sql_scripts/lab_test_names.sql @@ -0,0 +1,5 @@ +select lab_test_definition_id as id, + name, + standardised_vocabulary as vocab +from star.lab_test_definition as ltd +where ltd.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') \ No newline at end of file From 56ceb529974f74ed9f52d70fd3851696cb58f02c Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Sun, 9 Aug 2026 16:54:34 +0100 Subject: [PATCH 04/62] Add and refine SQL scripts for EHR data extraction Introduces new queries for airway and sputum/secretions data, and refactors the flow sheet query to consolidate multiple values into a wider format. A new README documents the overall data extraction goal and the current set of scripts. --- sql_scripts/README.md | 33 ++++++++++++++++++ sql_scripts/flow_sheet_values.sql | 50 +++++++++++++++++---------- sql_scripts/get_hospital_visit_id.sql | 7 +++- sql_scripts/lab_results.sql | 10 +++--- 4 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 sql_scripts/README.md diff --git a/sql_scripts/README.md b/sql_scripts/README.md new file mode 100644 index 0000000..98ae5af --- /dev/null +++ b/sql_scripts/README.md @@ -0,0 +1,33 @@ +# Notes on putting together the EHR needed + +## Goal + +The ultimate aim is to have one csv per patient per day which looks roughly like + + | DateTimeRecorded | Temperature | noradrenaline | etc | Secretions | etc | Placementinstant | RemovalInstant | TubeSize | etc |Units | Comments | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | +| 08/08/2026 00:00:15 | 36.4 | | | | | | | | | | +| 08/08/2026 00:00:16 | | 1 | | | | | | | | mg/L | | +| 08/08/2026 00:00:17 | | | | None | | | | | | | +| 08/08/2026 00:02:18 | | | | | | 07/08/2026 | |8mm | | | +| 08/08/2026 00:00:15 | 38.5 | | | | | | | | | | Doctors alerted | + +*Note: The insertion date for a tube may well be earlier than the day on which it is recorded as these seem to get populated during the nightly update to caboodle.* + +## Current scripts + +| script | arguments | record | location of script in repo | database | +|- | --- | --- |- | --- | +| mrn_based_on_bed_and_datetime.sql | location string | csn |waveform-controller/src/sql | star | +| get_hospital_visit_id.sql| csn | hospital_visit_id | waveform-controller/sql_scripts| star | +| flow_sheet_values.sql| hospital_visit_id/today/yesterday | part of table above | waveform-controller/sql_scripts| star | +| airway.sql | csn/today/yesterday | part of the table above | waveform-controller/sql_scripts | caboodle | +| sputum_secretions.sql | csn/today/yesterday | part of the table above | waveform-controller/sql_scripts | caboodle | + +## Unfinished scripts + +lab_results.sql need dealing with in the same way as flow_sheet_values + +lab_test_names.sql forms part of the above query but is useful for exploring + +We need scripts for any of the items in the a tracker that have not yet been covered. diff --git a/sql_scripts/flow_sheet_values.sql b/sql_scripts/flow_sheet_values.sql index 2a6c9eb..af9b76f 100644 --- a/sql_scripts/flow_sheet_values.sql +++ b/sql_scripts/flow_sheet_values.sql @@ -1,23 +1,35 @@ ---get the flow sheet values for the particular visit +--get the flow sheet values for the particular visit on a particular day -- the flow sheet numbers are recorded as id_in_application in the visit_observation_type table -- Temperature 6 -- Noradrenalin 3040102622 -- Metaraminol 12946 --- Secretion amount 451120 --- Sputum amount 302600 - - -select observation_datetime as datetime, -(select display_name -from star.visit_observation_type as vt -where vt.visit_observation_type_id = vo.visit_observation_type_id ) as observation, -value_as_real, -unit, -value_as_text, -comment, -vo.visit_observation_type_id -from star.visit_observation as vo -left join star.visit_observation_type as vt -on vo.visit_observation_type_id = vt.visit_observation_type_id -where vt.id_in_application in ('6', '12946', '302600', '451120', '3040102622') -and hospital_visit_id = 'xx' \ No newline at end of file + +SELECT + vo.observation_datetime AS DateTimeRecorded, + + (array_agg(vo.value_as_real) FILTER ( + WHERE vt.id_in_application = '6' + ))[1] AS "Temperature", + + (array_agg(vo.value_as_real) FILTER ( + WHERE vt.id_in_application = '3040102622' + ))[1] AS "Noradrenaline", + + (array_agg(vo.value_as_real) FILTER ( + WHERE vt.id_in_application = '12946' + ))[1] AS "Metaraminol", + + vo.unit AS Units, + vo.comment AS Comments + +FROM star.visit_observation AS vo + +LEFT JOIN star.visit_observation_type AS vt + ON vo.visit_observation_type_id = vt.visit_observation_type_id + +WHERE vt.id_in_application IN ('6', '3040102622', '12946') +AND vo.valid_from BETWEEN %(yesterday)s AND %(today)s +AND vo.hospital_visit_id = %(hospital_visit_id)s + +GROUP BY DateTimeRecorded, Units, vo.comment + diff --git a/sql_scripts/get_hospital_visit_id.sql b/sql_scripts/get_hospital_visit_id.sql index 5a60221..408447f 100644 --- a/sql_scripts/get_hospital_visit_id.sql +++ b/sql_scripts/get_hospital_visit_id.sql @@ -1,2 +1,7 @@ + + + select hospital_visit_id from star.hospital_visit as hv -where hv.encounter = 'csn' \ No newline at end of file +where hv.encounter = %(csn)s -- note the CSN must be in quotes + + diff --git a/sql_scripts/lab_results.sql b/sql_scripts/lab_results.sql index 9309722..aa05264 100644 --- a/sql_scripts/lab_results.sql +++ b/sql_scripts/lab_results.sql @@ -6,13 +6,13 @@ -- 390793060 WCC select -result_last_modified_datetime as date, +r.result_last_modified_datetime as date, (select name from star.lab_test_definition as ltd where ltd.lab_test_definition_id = r.lab_test_definition_id) as name, -value_as_real as value, -units, -abnormal_flag, -comment +r.value_as_real as value, +r.units, +r.abnormal_flag, +r.comment from star.lab_result as r From de65f8def5ebfc1e8f39f8800c3acd40dba6318b Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Mon, 24 Aug 2026 11:13:57 +0100 Subject: [PATCH 05/62] Refactor lab results query for wide format output Transforms specific lab test results (e.g., CRP, WCC) from a long format into distinct columns using `MAX() FILTER`. This enables direct consumption of lab data in a wider format, simplifying downstream analysis. Additionally, the query is updated to use parameters for `hospital_visit_id` and to include date range filtering, improving its flexibility. --- sql_scripts/lab_results.sql | 41 +++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/sql_scripts/lab_results.sql b/sql_scripts/lab_results.sql index aa05264..21b8b5d 100644 --- a/sql_scripts/lab_results.sql +++ b/sql_scripts/lab_results.sql @@ -1,24 +1,35 @@ --- This selects the values are units of lab tests +-- This selects the values of lab tests -- 1011 CRP -- 722790196 CRP -- 390793054 WCC -- 390793057 WCC -- 390793060 WCC -select -r.result_last_modified_datetime as date, -(select name from star.lab_test_definition as ltd -where ltd.lab_test_definition_id = r.lab_test_definition_id) as name, -r.value_as_real as value, -r.units, -r.abnormal_flag, -r.comment +SELECT + r.result_last_modified_datetime AS DateTimeRecorded, + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '1001') AS "C-reactive protein", + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793054') AS "CSF WCC TUBE 1", + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793057') AS "CSF WCC TUBE 2", + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793060') AS "CSF WCC TUBE 3", + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '722790196') AS "C-reactive protein" -from star.lab_result as r -join star.lab_order as o -on r.lab_order_id = o.lab_order_id -where o.hospital_visit_id = 'xx' -and r.result_status like 'FINAL' -and r.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') + r.units AS Units, + r.abnormal_flag AS Abnormal_result, + r.comment AS Comments +FROM star.lab_result AS r +LEFT JOIN star.lab_order AS o + ON r.lab_order_id = o.lab_order_id + +WHERE r.result_status like 'FINAL' +AND +r.lab_test_definition_id IN ('1001', + '390793054', + '390793057', + '390793060', + '722790196') +AND vo.valid_from BETWEEN %(yesterday)s AND %(today)s +AND o.hospital_visit_id = %(hospital_visit_id)s + +GROUP BY DateTimeRecorded, Units, Abnormal_result, Comments From 4426711ec662e8279c60ceda5db354028144734e Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 11 Aug 2026 15:42:19 +0100 Subject: [PATCH 06/62] Started on functions to wrap snakemake around sql calls --- src/electronic_health_records/__init__.py | 0 src/electronic_health_records/ehr.py | 180 ++++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 src/electronic_health_records/__init__.py create mode 100644 src/electronic_health_records/ehr.py diff --git a/src/electronic_health_records/__init__.py b/src/electronic_health_records/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py new file mode 100644 index 0000000..ebd514c --- /dev/null +++ b/src/electronic_health_records/ehr.py @@ -0,0 +1,180 @@ +import argparse +import functools +import json +import logging +from decimal import Decimal +from pathlib import Path +from typing import Any + +import pandas as pd +import pyarrow as pa +import pyarrow.parquet as pq +import settings + +from locations import ( + CSV_PATTERN, + ORIGINAL_PARQUET_PATTERN, + PSEUDONYMISED_PARQUET_PATTERN, +) +from .hashing import do_hash + +def ehr_for_csv( + *, + date_str: str, + original_csn: str, + hashed_csn: str, +) -> None: + """Extracts electronic healthcare records for a given csn and writes the + results to a pseudonymised csv file for a single day. + + This is a privacy-sensitive area of code. Unhashed CSNs must not appear in + uploaded files. + :param date_str: the date to look up data for + :param original_csn: the csn to base look up on. + :param hashed_csn: the pseudonymised hash to use for file output. + + """ + # will pick up the logger config defined in the snakemake job (ie. log to file) + logger = logging.getLogger(__name__) + + # it's in the csv_path and original_parquet_path, but at least nowhere else! + del original_csn + + logger.info("Turning CSV %s to parquets", csv_path) + csv_path.parent.mkdir(parents=True, exist_ok=True) + original_parquet_path.parent.mkdir(parents=True, exist_ok=True) + df = pd.read_csv( + str(csv_path), + dtype={ + "csn": str, + "mrn": str, + "source_variable_id": str, + "source_channel_id": str, + "units": str, + "sampling_rate": int, + "timestamp": float, + "location": str, + "values": str, + }, + header=0, # the first line is always the header + ) + + def parse_array(x): + # Not sure if this is the most efficient way. Might be able to do something with DecimalArray? + # return [pa.decimal128(i) for i in x.replace(' ', '').split(',')] + return [Decimal(i) for i in x.strip().strip("[]").replace(" ", "").split(",")] + + df["values"] = df["values"].apply(parse_array) + + # Convert pandas DataFrame to pyarrow Table with proper types + schema = pa.schema( + [ + ("csn", pa.string()), + ("mrn", pa.string()), + ("source_variable_id", pa.string()), + ("source_channel_id", pa.string()), + ("units", pa.string()), + ("sampling_rate", pa.int32()), + ("timestamp", pa.float64()), + ("location", pa.string()), + # As per requirements, compactness is important here. + # decimal32 can have a maximum of 9 significant digits and should + # satisfy our needs, but it only exists in pyarrow >= 19. + # We are currently tied to 18.1 because of PIXL core. + # So for now, use decimal128 instead. + # Not yet tested whether the specified precision + # and scale cause it to be equivalent in size to decimal32. + # See issue #31. + ("values", pa.list_(pa.decimal128(9, 4))), + ] + ) + table = pa.Table.from_pandas(df, schema=schema, preserve_index=True) + + # mark the parquet files themselves as production or not. + our_metadata = {"instance_name": settings.INSTANCE_NAME} + + table = add_waveform_metadata_to_table(table, our_metadata) + + pq.write_table( + table, + str(original_parquet_path), + # valid values: {‘NONE’, ‘SNAPPY’, ‘GZIP’, ‘BROTLI’, ‘LZ4’, ‘ZSTD’} + compression="zstd", + use_dictionary=True, + write_statistics=True, # enable indexes/statistics + flavor="spark", + ) + logger.info( + "Done turning CSV %s to original parquet %s", csv_path, original_parquet_path + ) + + df = pseudonymise_relevant_columns(df) + pseudon_table = pa.Table.from_pandas(df, schema=schema, preserve_index=True) + + # Use same metadata for pseudon, must not contain identifiers! + pseudon_table = add_waveform_metadata_to_table(pseudon_table, our_metadata) + + hashed_path = Path( + str(PSEUDONYMISED_PARQUET_PATTERN).format( + date=date_str, + hashed_csn=hashed_csn, + variable_id=variable_id, + channel_id=channel_id, + units=units, + ) + ) + pq.write_table( + pseudon_table, + str(hashed_path), + compression="zstd", + use_dictionary=True, + write_statistics=True, # enable indexes/statistics + flavor="spark", + ) + logger.info( + "Done turning CSV %s to pseudonymised parquet %s", csv_path, hashed_path + ) + + +def add_waveform_metadata_to_table( + existing_table: pa.Table, metadata: dict[str, Any] +) -> pa.Table: + """Replace our metadata in its entirety, leaving untouched metadata we didn't + set.""" + + # Parquet footer metadata is a series of (byte string) key-value pairs. + # Other users of metadata (eg. pandas) convert their metadata to JSON and store it under + # a single key (a namespace, effectively), so we'll do the same under our own key. + waveform_exporter_metadata_key = b"waveform_exporter" + + existing_metadata = existing_table.schema.metadata or {} + json_byte_string = json.dumps(metadata).encode("utf-8") + existing_table = existing_table.replace_schema_metadata( + {**existing_metadata, waveform_exporter_metadata_key: json_byte_string} + ) + return existing_table + + +SAFE_COLUMNS = [ + "sampling_rate", + "source_variable_id", + "source_channel_id", + "timestamp", + "units", + "values", +] + + +def pseudonymise_relevant_columns(df: pd.DataFrame): + """ "csn", "mrn", "location" are examples of columns that must be pseudonymised. + + However, it's safer to list which columns *don't* need to be pseudonymised. Eg. you + add a column but forget to consider whether it's sensitive, OR you rename one of the + known sensitive columns and forget that this will cause privacy to break. This means + that when you add a new column, you have to add it here if you don't want it to be + hashed. + """ + for col in df.columns: + if col not in SAFE_COLUMNS: + df[col] = df[col].apply(functools.partial(do_hash, col)) + return df From 1b8545af40970358acffd097b57dd40766746a53 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 12 Aug 2026 14:13:13 +0100 Subject: [PATCH 07/62] Started to implement caboodle look up code --- src/db.py | 44 ++++++++ src/electronic_health_records/ehr.py | 153 ++++----------------------- 2 files changed, 64 insertions(+), 133 deletions(-) diff --git a/src/db.py b/src/db.py index ebab871..dff5872 100644 --- a/src/db.py +++ b/src/db.py @@ -53,3 +53,47 @@ def get_row(self, location_string: str, observation_datetime: datetime): ) return rows[0] + + +class caboodleDB: + """For querying the caboodle database to extract electronic healthcare records per + patient.""" + + connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( + settings.CABOODLE_DBNAME, # type:ignore + settings.CABOODLE_USERNAME, # type:ignore + settings.CABOODLE_PASSWORD, # type:ignore + settings.CABOODLE_HOST, # type:ignore + settings.CABOODLE_PORT, # type:ignore + settings.CABOODLE_CONNECT_TIMEOUT, # type:ignore + settings.CABOODLE_QUERY_TIMEOUT, # type:ignore + ) + connection_pool: pool.ThreadedConnectionPool + + def connect(self): + """Set up connection to the database.""" + self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) + + def get_airflow(self, start_datetime: datetime, end_datetime: datetime, csn: str): + """Retrieve airflow data from database.""" + with open("src/sql/airway.sql", "r") as file: + airway_query = sql.SQL(file.read()) + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "csn": csn, + } + return self._get_rows(airway_query, parameters) + + def _get_rows(self, sql_query: sql.SQL, parameters: dict): + try: + with self.connection_pool.getconn() as db_connection: + with db_connection.cursor() as curs: + curs.execute(sql_query, parameters) + rows = curs.fetchall() + self.connection_pool.putconn(db_connection) + except psycopg2.errors.OperationalError as e: + self.connection_pool.putconn(db_connection) + raise ConnectionError(f"Data base error: {e}") + + return rows diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index ebd514c..a5d063a 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -1,158 +1,45 @@ -import argparse import functools -import json import logging -from decimal import Decimal -from pathlib import Path -from typing import Any import pandas as pd -import pyarrow as pa -import pyarrow.parquet as pq -import settings +from datetime import datetime -from locations import ( - CSV_PATTERN, - ORIGINAL_PARQUET_PATTERN, - PSEUDONYMISED_PARQUET_PATTERN, -) from .hashing import do_hash +from db import caboodleDB + def ehr_for_csv( - *, - date_str: str, + datetime: datetime, original_csn: str, hashed_csn: str, + db_connection: caboodleDB, ) -> None: - """Extracts electronic healthcare records for a given csn and writes the - results to a pseudonymised csv file for a single day. + """Extracts electronic healthcare records for a given csn and writes the results to + a pseudonymised csv file for a single day. - This is a privacy-sensitive area of code. Unhashed CSNs must not appear in - uploaded files. + This is a privacy-sensitive area of code. Unhashed CSNs must not appear in uploaded + files. :param date_str: the date to look up data for :param original_csn: the csn to base look up on. :param hashed_csn: the pseudonymised hash to use for file output. - + :param db_connection: connection to the caboodle database. """ # will pick up the logger config defined in the snakemake job (ie. log to file) logger = logging.getLogger(__name__) - # it's in the csv_path and original_parquet_path, but at least nowhere else! - del original_csn - - logger.info("Turning CSV %s to parquets", csv_path) - csv_path.parent.mkdir(parents=True, exist_ok=True) - original_parquet_path.parent.mkdir(parents=True, exist_ok=True) - df = pd.read_csv( - str(csv_path), - dtype={ - "csn": str, - "mrn": str, - "source_variable_id": str, - "source_channel_id": str, - "units": str, - "sampling_rate": int, - "timestamp": float, - "location": str, - "values": str, - }, - header=0, # the first line is always the header - ) - - def parse_array(x): - # Not sure if this is the most efficient way. Might be able to do something with DecimalArray? - # return [pa.decimal128(i) for i in x.replace(' ', '').split(',')] - return [Decimal(i) for i in x.strip().strip("[]").replace(" ", "").split(",")] - - df["values"] = df["values"].apply(parse_array) - - # Convert pandas DataFrame to pyarrow Table with proper types - schema = pa.schema( - [ - ("csn", pa.string()), - ("mrn", pa.string()), - ("source_variable_id", pa.string()), - ("source_channel_id", pa.string()), - ("units", pa.string()), - ("sampling_rate", pa.int32()), - ("timestamp", pa.float64()), - ("location", pa.string()), - # As per requirements, compactness is important here. - # decimal32 can have a maximum of 9 significant digits and should - # satisfy our needs, but it only exists in pyarrow >= 19. - # We are currently tied to 18.1 because of PIXL core. - # So for now, use decimal128 instead. - # Not yet tested whether the specified precision - # and scale cause it to be equivalent in size to decimal32. - # See issue #31. - ("values", pa.list_(pa.decimal128(9, 4))), - ] - ) - table = pa.Table.from_pandas(df, schema=schema, preserve_index=True) - - # mark the parquet files themselves as production or not. - our_metadata = {"instance_name": settings.INSTANCE_NAME} + caboodle = caboodleDB() + caboodle.connect() - table = add_waveform_metadata_to_table(table, our_metadata) + logger.info("Looking for airway data for %s.", hashed_csn) - pq.write_table( - table, - str(original_parquet_path), - # valid values: {‘NONE’, ‘SNAPPY’, ‘GZIP’, ‘BROTLI’, ‘LZ4’, ‘ZSTD’} - compression="zstd", - use_dictionary=True, - write_statistics=True, # enable indexes/statistics - flavor="spark", - ) - logger.info( - "Done turning CSV %s to original parquet %s", csv_path, original_parquet_path - ) + start_datetime = datetime + end_datetime = datetime + airflow = caboodle.get_airflow(start_datetime, end_datetime, original_csn) + airflow = pseudonymise_relevant_columns(airflow) + logger.info(airflow) - df = pseudonymise_relevant_columns(df) - pseudon_table = pa.Table.from_pandas(df, schema=schema, preserve_index=True) - - # Use same metadata for pseudon, must not contain identifiers! - pseudon_table = add_waveform_metadata_to_table(pseudon_table, our_metadata) - - hashed_path = Path( - str(PSEUDONYMISED_PARQUET_PATTERN).format( - date=date_str, - hashed_csn=hashed_csn, - variable_id=variable_id, - channel_id=channel_id, - units=units, - ) - ) - pq.write_table( - pseudon_table, - str(hashed_path), - compression="zstd", - use_dictionary=True, - write_statistics=True, # enable indexes/statistics - flavor="spark", - ) - logger.info( - "Done turning CSV %s to pseudonymised parquet %s", csv_path, hashed_path - ) - - -def add_waveform_metadata_to_table( - existing_table: pa.Table, metadata: dict[str, Any] -) -> pa.Table: - """Replace our metadata in its entirety, leaving untouched metadata we didn't - set.""" - - # Parquet footer metadata is a series of (byte string) key-value pairs. - # Other users of metadata (eg. pandas) convert their metadata to JSON and store it under - # a single key (a namespace, effectively), so we'll do the same under our own key. - waveform_exporter_metadata_key = b"waveform_exporter" - - existing_metadata = existing_table.schema.metadata or {} - json_byte_string = json.dumps(metadata).encode("utf-8") - existing_table = existing_table.replace_schema_metadata( - {**existing_metadata, waveform_exporter_metadata_key: json_byte_string} - ) - return existing_table + # delete csn once we no longer need it + del original_csn SAFE_COLUMNS = [ From a8544920804c57a500a2804d450db9e9c35ee8bc Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 12 Aug 2026 14:52:10 +0100 Subject: [PATCH 08/62] Setting up config and docker bits --- config.EXAMPLE/ehr_lookup.EXAMPLE | 9 +++++++++ docker-compose.yml | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 config.EXAMPLE/ehr_lookup.EXAMPLE diff --git a/config.EXAMPLE/ehr_lookup.EXAMPLE b/config.EXAMPLE/ehr_lookup.EXAMPLE new file mode 100644 index 0000000..022ce93 --- /dev/null +++ b/config.EXAMPLE/ehr_lookup.EXAMPLE @@ -0,0 +1,9 @@ +# This is an EXAMPLE file, do not put real secrets in here. +# Copy it to ../config/ehr_lookup.env and then DELETE THIS COMMENT. +CABOODLE_DBNAME="fakecab" +CABOODLE_USERNAME="inform_user" +CABOODLE_PASSWORD="inform" +CABOODLE_HOST="localhost" +CABOODLE_PORT="5433" +CABOODLE_CONNECT_TIMEOUT="10" # in seconds +CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds diff --git a/docker-compose.yml b/docker-compose.yml index 6c30ca7..eaaa113 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -52,3 +52,24 @@ services: env_file: - ../config/hasher.env restart: unless-stopped + ehr-lookup: + build: + context: .. + dockerfile: waveform-controller/Dockerfile + target: ehr-lookup + args: + HTTP_PROXY: ${HTTP_PROXY} + http_proxy: ${http_proxy} + HTTPS_PROXY: ${HTTPS_PROXY} + https_proxy: ${https_proxy} + # ideally we'd use docker secrets but it's not enabled currently + env_file: + - ../config/ehr_lookup.env + volumes: + - ../waveform-export:/waveform-export + # because we're launching through cron in the container, which starts + # processes with a clean environment, also mount in the config file so + # it can be read in by snakemake later + - ../config/ehr-lookup.env:/config/ehr-lookup.env:ro + restart: unless-stopped + From 77848544d4a61d4f77e05974c59e5f419e8b39ad Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 17 Aug 2026 11:44:28 +0100 Subject: [PATCH 09/62] Add a ehr location to snakefile --- src/pipeline/Snakefile | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pipeline/Snakefile b/src/pipeline/Snakefile index dbce7db..7dc9a3a 100644 --- a/src/pipeline/Snakefile +++ b/src/pipeline/Snakefile @@ -9,6 +9,7 @@ from locations import ( WAVEFORM_ORIGINAL_CSV, WAVEFORM_SNAKEMAKE_LOGS, WAVEFORM_PSEUDONYMISED_PARQUET, + WAVEFORM_PSEUDONYMISED_EHR, WAVEFORM_FTPS_LOGS, HASH_LOOKUP_JSON, HASH_LOOKUP_JSON_REL, From bdf0eb273f5cf7c2d935072b14896477aca02d73 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 17 Aug 2026 15:39:19 +0100 Subject: [PATCH 10/62] Tidied up implementation and rewrote Snakefile logic, with help from Claude Sonnet 5.0 --- src/csv_writer.py | 28 ++++++++++- src/electronic_health_records/ehr.py | 73 ++++++++++++++-------------- src/locations.py | 3 ++ src/pipeline/Snakefile | 38 ++++++++++++++- src/pipeline/utils.py | 6 +++ src/pseudon/pseudon.py | 25 +++++----- 6 files changed, 121 insertions(+), 52 deletions(-) diff --git a/src/csv_writer.py b/src/csv_writer.py index 66948b8..42daeb3 100644 --- a/src/csv_writer.py +++ b/src/csv_writer.py @@ -2,8 +2,15 @@ import csv from datetime import datetime +import pandas as pd -from locations import WAVEFORM_ORIGINAL_CSV, make_file_name, FILE_STEM_PATTERN +from locations import ( + WAVEFORM_ORIGINAL_CSV, + WAVEFORM_PSEUDONYMISED_EHR, + make_file_name, + FILE_STEM_PATTERN, + EHR_STEM_PATTERN_HASHED, +) def create_file_name( @@ -77,3 +84,22 @@ def write_frame( ) return True + + +def write_ehr( + df: pd.DataFrame, + date_str: str, + hashed_csn: str, +) -> bool: + """Writes a frame of electronic healthcare data to a csv file. + + :return: True if write was successful. + """ + subs_dict = dict(date=date_str, hashed_csn=hashed_csn) + stem = make_file_name(EHR_STEM_PATTERN_HASHED, subs_dict) + filename = WAVEFORM_PSEUDONYMISED_EHR / f"{stem}_ehr.csv" + filename.parent.mkdir(exist_ok=True, parents=True) + + df.to_csv(filename) + + return True diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index a5d063a..48fc930 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -1,15 +1,31 @@ -import functools import logging -import pandas as pd -from datetime import datetime +from datetime import datetime, timedelta -from .hashing import do_hash from db import caboodleDB +from csv_writer import write_ehr +from pseudon.pseudon import pseudonymise_relevant_columns -def ehr_for_csv( - datetime: datetime, +def ehr_for_csv(date_str: str, original_csn: str, hashed_csn: str) -> None: + """Extracts electronic healthcare records for a given csn and writes the results to + a pseudonymised csv file for a single day. + + This is a privacy-sensitive area of code. Unhashed CSNs must not appear in uploaded + files. + :param date_str: the date to look up data for + :param original_csn: the csn to base look up on. + :param hashed_csn: the pseudonymised hash to use for file output. + """ + + db_connection = caboodleDB() + db_connection.connect() + + _ehr_for_csv(date_str, original_csn, hashed_csn, db_connection) + + +def _ehr_for_csv( + date_str: str, original_csn: str, hashed_csn: str, db_connection: caboodleDB, @@ -27,41 +43,24 @@ def ehr_for_csv( # will pick up the logger config defined in the snakemake job (ie. log to file) logger = logging.getLogger(__name__) - caboodle = caboodleDB() - caboodle.connect() - logger.info("Looking for airway data for %s.", hashed_csn) - start_datetime = datetime - end_datetime = datetime - airflow = caboodle.get_airflow(start_datetime, end_datetime, original_csn) - airflow = pseudonymise_relevant_columns(airflow) - logger.info(airflow) + start_datetime = datetime.strptime(date_str, "%Y-%m-%d") + end_datetime = start_datetime + timedelta(days=1) + airflow = db_connection.get_airflow(start_datetime, end_datetime, original_csn) - # delete csn once we no longer need it - del original_csn + safe_columns = [ + "DateTimeRecorded", + "PlacementInstant", + "RemovalInstant", + "TubeSize", + ] + airflow = pseudonymise_relevant_columns(airflow, safe_columns) -SAFE_COLUMNS = [ - "sampling_rate", - "source_variable_id", - "source_channel_id", - "timestamp", - "units", - "values", -] + write_ehr(airflow, date_str, hashed_csn) + logger.info(airflow) -def pseudonymise_relevant_columns(df: pd.DataFrame): - """ "csn", "mrn", "location" are examples of columns that must be pseudonymised. - - However, it's safer to list which columns *don't* need to be pseudonymised. Eg. you - add a column but forget to consider whether it's sensitive, OR you rename one of the - known sensitive columns and forget that this will cause privacy to break. This means - that when you add a new column, you have to add it here if you don't want it to be - hashed. - """ - for col in df.columns: - if col not in SAFE_COLUMNS: - df[col] = df[col].apply(functools.partial(do_hash, col)) - return df + # delete csn once we no longer need it + del original_csn diff --git a/src/locations.py b/src/locations.py index bb15847..f55fa20 100644 --- a/src/locations.py +++ b/src/locations.py @@ -5,6 +5,7 @@ WAVEFORM_ORIGINAL_PARQUET = WAVEFORM_EXPORT_BASE / "original-parquet" WAVEFORM_HASH_LOOKUPS = WAVEFORM_EXPORT_BASE / "hash-lookups" WAVEFORM_PSEUDONYMISED_PARQUET = WAVEFORM_EXPORT_BASE / "pseudonymised" +WAVEFORM_PSEUDONYMISED_EHR = WAVEFORM_EXPORT_BASE / "pseudonymised_ehr" WAVEFORM_SNAKEMAKE_LOGS = WAVEFORM_EXPORT_BASE / "snakemake-logs" WAVEFORM_FTPS_LOGS = WAVEFORM_EXPORT_BASE / "ftps-logs" @@ -14,6 +15,8 @@ FILE_STEM_PATTERN_HASHED = ( "{date}/{date}.{hashed_csn}.{variable_id}.{channel_id}.{units}" ) +# EHR data is per (date, csn), not per variable/channel/units, so it gets its own stem. +EHR_STEM_PATTERN_HASHED = "{date}/{date}.{hashed_csn}" CSV_PATTERN = WAVEFORM_ORIGINAL_CSV / (FILE_STEM_PATTERN + ".csv") ORIGINAL_PARQUET_PATTERN = WAVEFORM_ORIGINAL_PARQUET / (FILE_STEM_PATTERN + ".parquet") PSEUDONYMISED_PARQUET_PATTERN = WAVEFORM_PSEUDONYMISED_PARQUET / ( diff --git a/src/pipeline/Snakefile b/src/pipeline/Snakefile index 7dc9a3a..71042d9 100644 --- a/src/pipeline/Snakefile +++ b/src/pipeline/Snakefile @@ -15,9 +15,11 @@ from locations import ( HASH_LOOKUP_JSON_REL, FILE_STEM_PATTERN, FILE_STEM_PATTERN_HASHED, + EHR_STEM_PATTERN_HASHED, make_file_name, ) from pseudon.pseudon import csv_to_parquets +from electronic_health_records.ehr import ehr_for_csv from utils import config_bool, determine_eventual_outputs @@ -48,6 +50,7 @@ PROCESS_CSV_FROM_DATE = str(config['PROCESS_CSV_FROM_DATE']) all_outputs, hash_to_csn = determine_eventual_outputs(CSV_AGE_THRESHOLD_MINUTES, ONLY_USE_CSV_FROM_YESTERDAY, PROCESS_CSV_FROM_DATE) ALL_FTPS_UPLOADED = [ao.get_ftps_uploaded_file() for ao in all_outputs] ALL_DAILY_HASH_LOOKUPS = sorted({ao.get_daily_hash_lookup() for ao in all_outputs}) +ALL_EHR_LOOKUPS = sorted({ao.get_ehr_lookup() for ao in all_outputs}) def configure_file_logging(log_file): import logging @@ -65,7 +68,8 @@ def configure_file_logging(log_file): rule all: input: ftps_uploaded = ALL_FTPS_UPLOADED, - daily_hash_lookups = ALL_DAILY_HASH_LOOKUPS + daily_hash_lookups = ALL_DAILY_HASH_LOOKUPS, + ehr_lookups = ALL_EHR_LOOKUPS rule all_ftps_uploaded: input: @@ -75,6 +79,10 @@ rule all_daily_hash_lookups: input: ALL_DAILY_HASH_LOOKUPS +rule all_ehr_lookups: + input: + ALL_EHR_LOOKUPS + def input_file_maker(wc): unhashed_csn = hash_to_csn[wc.hashed_csn] # when using input functions, snakemake doesn't do its normal templating, you have to do it, hence the f-string @@ -116,6 +124,34 @@ def pseudonymised_parquet_files_for_date(wc): return [ao.get_pseudonymised_parquet_path() for ao in all_outputs if ao.date == wc.date] +def pseudonymised_parquet_files_for_date_and_hashed_csn(wc): + return [ + ao.get_pseudonymised_parquet_path() + for ao in all_outputs + if ao.date == wc.date and ao.hashed_csn == wc.hashed_csn + ] + + +rule ehr_lookup: + input: + # As with daily_hash_lookup, we lie to Snakemake that the input is the pseudon + # parquets for this csn/day, purely so this rule is tied into the dependency DAG + # and reruns if the underlying data for this csn/day changes. + pseudonymised_parquets = pseudonymised_parquet_files_for_date_and_hashed_csn + output: + WAVEFORM_PSEUDONYMISED_EHR / (EHR_STEM_PATTERN_HASHED + "_ehr.csv") + log: + WAVEFORM_SNAKEMAKE_LOGS / "ehr_lookup" / (EHR_STEM_PATTERN_HASHED + ".log") + run: + logger = configure_file_logging(log[0]) + original_csn = hash_to_csn[wildcards.hashed_csn] + logger.info("Running EHR look up for csn %s. Hash -> %s", original_csn, wildcards.hashed_csn) + ehr_for_csv( + date_str=wildcards.date, + original_csn=original_csn, + hashed_csn=wildcards.hashed_csn) + + rule daily_hash_lookup: input: # Because we don't declare the original parquets in the output of csv_to_parquet, diff --git a/src/pipeline/utils.py b/src/pipeline/utils.py index 6bf58b4..a17cd89 100644 --- a/src/pipeline/utils.py +++ b/src/pipeline/utils.py @@ -8,10 +8,12 @@ from pseudon.hashing import do_hash from locations import ( WAVEFORM_PSEUDONYMISED_PARQUET, + WAVEFORM_PSEUDONYMISED_EHR, WAVEFORM_FTPS_LOGS, HASH_LOOKUP_JSON, ORIGINAL_PARQUET_PATTERN, FILE_STEM_PATTERN_HASHED, + EHR_STEM_PATTERN_HASHED, CSV_PATTERN, make_file_name, ) @@ -73,6 +75,10 @@ def get_ftps_uploaded_file(self) -> Path: def get_daily_hash_lookup(self) -> Path: return Path(make_file_name(str(HASH_LOOKUP_JSON), self._subs_dict)) + def get_ehr_lookup(self) -> Path: + final_stem = make_file_name(EHR_STEM_PATTERN_HASHED, self._subs_dict) + return WAVEFORM_PSEUDONYMISED_EHR / f"{final_stem}_ehr.csv" + def get_file_age(file_path: Path) -> timedelta: # need to use UTC to avoid DST issues diff --git a/src/pseudon/pseudon.py b/src/pseudon/pseudon.py index 01cc498..485d310 100644 --- a/src/pseudon/pseudon.py +++ b/src/pseudon/pseudon.py @@ -142,7 +142,16 @@ def parse_array(x): "Done turning CSV %s to original parquet %s", csv_path, original_parquet_path ) - df = pseudonymise_relevant_columns(df) + safe_columns = [ + "sampling_rate", + "source_variable_id", + "source_channel_id", + "timestamp", + "units", + "values", + ] + + df = pseudonymise_relevant_columns(df, safe_columns) pseudon_table = pa.Table.from_pandas(df, schema=schema, preserve_index=True) # Use same metadata for pseudon, must not contain identifiers! @@ -189,17 +198,7 @@ def add_waveform_metadata_to_table( return existing_table -SAFE_COLUMNS = [ - "sampling_rate", - "source_variable_id", - "source_channel_id", - "timestamp", - "units", - "values", -] - - -def pseudonymise_relevant_columns(df: pd.DataFrame): +def pseudonymise_relevant_columns(df: pd.DataFrame, safe_columns: [str]): """ "csn", "mrn", "location" are examples of columns that must be pseudonymised. However, it's safer to list which columns *don't* need to be pseudonymised. Eg. you @@ -209,6 +208,6 @@ def pseudonymise_relevant_columns(df: pd.DataFrame): hashed. """ for col in df.columns: - if col not in SAFE_COLUMNS: + if col not in safe_columns: df[col] = df[col].apply(functools.partial(do_hash, col)) return df From f4382d2f342ace60b1b206b93f820b3760d08c43 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 17 Aug 2026 15:53:06 +0100 Subject: [PATCH 11/62] We don't need a separate container for ehr lookup --- docker-compose.yml | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index eaaa113..6c30ca7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -52,24 +52,3 @@ services: env_file: - ../config/hasher.env restart: unless-stopped - ehr-lookup: - build: - context: .. - dockerfile: waveform-controller/Dockerfile - target: ehr-lookup - args: - HTTP_PROXY: ${HTTP_PROXY} - http_proxy: ${http_proxy} - HTTPS_PROXY: ${HTTPS_PROXY} - https_proxy: ${https_proxy} - # ideally we'd use docker secrets but it's not enabled currently - env_file: - - ../config/ehr_lookup.env - volumes: - - ../waveform-export:/waveform-export - # because we're launching through cron in the container, which starts - # processes with a clean environment, also mount in the config file so - # it can be read in by snakemake later - - ../config/ehr-lookup.env:/config/ehr-lookup.env:ro - restart: unless-stopped - From de12629a216dc5b7a7646a3397cfbffd7de627d6 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 17 Aug 2026 16:00:46 +0100 Subject: [PATCH 12/62] Added ehr variables to settings and added a testing flag. --- config.EXAMPLE/ehr_lookup.EXAMPLE | 5 +++++ src/settings.py | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/config.EXAMPLE/ehr_lookup.EXAMPLE b/config.EXAMPLE/ehr_lookup.EXAMPLE index 022ce93..0c88d12 100644 --- a/config.EXAMPLE/ehr_lookup.EXAMPLE +++ b/config.EXAMPLE/ehr_lookup.EXAMPLE @@ -7,3 +7,8 @@ CABOODLE_HOST="localhost" CABOODLE_PORT="5433" CABOODLE_CONNECT_TIMEOUT="10" # in seconds CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds + +# To avoid having to deploy a fake caboodle for testing we have +# a testing flag for Caboodle. If set TRUE caboodle connection will +# fail silently and ehr file will be created with fake data +CABOODLE_TESTING="FALSE" diff --git a/src/settings.py b/src/settings.py index 3bccd12..0309839 100644 --- a/src/settings.py +++ b/src/settings.py @@ -37,6 +37,15 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("HASHER_API_HOSTNAME") get_from_env("HASHER_API_PORT") +get_from_env("CABOODLE_DBNAME") +get_from_env("CABOODLE_USERNAME") +get_from_env("CABOODLE_PASSWORD") +get_from_env("CABOODLE_HOST") +get_from_env("CABOODLE_PORT") +get_from_env("CABOODLE_CONNECT_TIMEOUT") +get_from_env("CABOODLE_QUERY_TIMEOUT") +get_from_env("CABOODLE_TESTING") + get_from_env("LOG_LEVEL", default_value="INFO") get_from_env("INSTANCE_NAME", required=True) From 314a613900f3e33e338c0a48c8a0230afa6d2589 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 17 Aug 2026 16:18:20 +0100 Subject: [PATCH 13/62] added some caboodle faking logic to enable local running --- src/db.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/db.py b/src/db.py index dff5872..e0fa11f 100644 --- a/src/db.py +++ b/src/db.py @@ -1,4 +1,5 @@ from datetime import datetime +import pandas as pd import psycopg2 from psycopg2 import sql, pool import logging @@ -69,13 +70,24 @@ class caboodleDB: settings.CABOODLE_QUERY_TIMEOUT, # type:ignore ) connection_pool: pool.ThreadedConnectionPool + fake_caboodle: bool def connect(self): """Set up connection to the database.""" - self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) + self.fake_caboodle = True if settings.CABOODLE_TESTING == "TRUE" else False + if not self.fake_caboodle: + self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) def get_airflow(self, start_datetime: datetime, end_datetime: datetime, csn: str): """Retrieve airflow data from database.""" + if self.fake_caboodle: + fake_airway = { + "DateTimeRecorded": [0], + "PlacementInstant": [0], + "RemovalInstant": [0], + "TubeSize": [0], + } + return pd.DataFrame(data=fake_airway) with open("src/sql/airway.sql", "r") as file: airway_query = sql.SQL(file.read()) parameters = { From f33621d47c5a28194f7c0ae056d6db642c137ed9 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 15:21:33 +0100 Subject: [PATCH 14/62] Caboodle setup goes into exporter.env --- config.EXAMPLE/ehr_lookup.EXAMPLE | 14 -------------- config.EXAMPLE/exporter.env.EXAMPLE | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) delete mode 100644 config.EXAMPLE/ehr_lookup.EXAMPLE diff --git a/config.EXAMPLE/ehr_lookup.EXAMPLE b/config.EXAMPLE/ehr_lookup.EXAMPLE deleted file mode 100644 index 0c88d12..0000000 --- a/config.EXAMPLE/ehr_lookup.EXAMPLE +++ /dev/null @@ -1,14 +0,0 @@ -# This is an EXAMPLE file, do not put real secrets in here. -# Copy it to ../config/ehr_lookup.env and then DELETE THIS COMMENT. -CABOODLE_DBNAME="fakecab" -CABOODLE_USERNAME="inform_user" -CABOODLE_PASSWORD="inform" -CABOODLE_HOST="localhost" -CABOODLE_PORT="5433" -CABOODLE_CONNECT_TIMEOUT="10" # in seconds -CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds - -# To avoid having to deploy a fake caboodle for testing we have -# a testing flag for Caboodle. If set TRUE caboodle connection will -# fail silently and ehr file will be created with fake data -CABOODLE_TESTING="FALSE" diff --git a/config.EXAMPLE/exporter.env.EXAMPLE b/config.EXAMPLE/exporter.env.EXAMPLE index 39a4119..c49d503 100644 --- a/config.EXAMPLE/exporter.env.EXAMPLE +++ b/config.EXAMPLE/exporter.env.EXAMPLE @@ -32,3 +32,17 @@ ONLY_USE_CSV_FROM_YESTERDAY=TRUE # specify a date to process format YYYY-MM-DD also accepts a regular # expression to match multiple date PROCESS_CSV_FROM_DATE= + +# We query Caboodle to get electronic healthcare record date per patient per day +CABOODLE_DBNAME="fakecab" +CABOODLE_USERNAME="inform_user" +CABOODLE_PASSWORD="inform" +CABOODLE_HOST="localhost" +CABOODLE_PORT="5433" +CABOODLE_CONNECT_TIMEOUT="10" # in seconds +CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds + +# To avoid having to deploy a fake caboodle for testing we have +# a testing flag for Caboodle. If set TRUE caboodle connection will +# fail silently and ehr file will be created with fake data +CABOODLE_TESTING="FALSE" From de875db509c5b1621147eb9f5278221ff1206f84 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 15:51:40 +0100 Subject: [PATCH 15/62] Format fix --- src/db.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/db.py b/src/db.py index e0fa11f..04eb37a 100644 --- a/src/db.py +++ b/src/db.py @@ -76,7 +76,9 @@ def connect(self): """Set up connection to the database.""" self.fake_caboodle = True if settings.CABOODLE_TESTING == "TRUE" else False if not self.fake_caboodle: - self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) + self.connection_pool = pool.SimpleConnectionPool( + 1, 1, self.connection_string + ) def get_airflow(self, start_datetime: datetime, end_datetime: datetime, csn: str): """Retrieve airflow data from database.""" From 43e9f542de3e7a37b9d604b98eaa7ba6ef5c8062 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 16:06:02 +0100 Subject: [PATCH 16/62] Beginning of test for ehr integration --- src/csv_writer.py | 2 +- tests/helpers.py | 3 +++ tests/test_snakemake_integration.py | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/csv_writer.py b/src/csv_writer.py index 42daeb3..11588b1 100644 --- a/src/csv_writer.py +++ b/src/csv_writer.py @@ -100,6 +100,6 @@ def write_ehr( filename = WAVEFORM_PSEUDONYMISED_EHR / f"{stem}_ehr.csv" filename.parent.mkdir(exist_ok=True, parents=True) - df.to_csv(filename) + df.to_csv(filename, index = False) return True diff --git a/tests/helpers.py b/tests/helpers.py index 63d82f2..68df5cc 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -51,6 +51,9 @@ def get_orig_parquet(self): def get_pseudon_parquet(self): return f"{self.date}/{self.date}.{self.get_hashed_csn()}.{self.variable_id}.{self.channel_id}.{self.units}.parquet" + def get_pseudon_ehr(self): + return f"{self.date}/{self.date}.{self.get_hashed_csn()}_ehr.csv" + def get_hashes(self): return f"{self.date}/{self.date}.hashes.json" diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 50d7b9c..3bf8a4d 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -207,9 +207,11 @@ def test_snakemake_pipeline(tmp_path: Path, background_hasher): tmp_path / "original-parquet" / filename.get_orig_parquet() ) pseudon_path = tmp_path / "pseudonymised" / filename.get_pseudon_parquet() + ehr_path = tmp_path / "pseudonymised_ehr" / filename.get_pseudon_ehr() assert original_parquet_path.exists() assert pseudon_path.exists() + assert ehr_path.exists() _compare_original_parquet_to_expected(original_parquet_path, expected_data) _compare_parquets(original_parquet_path, pseudon_path) From 21e0e5b3dfa7e2bfb9cae27f9f5fc07adeadf58d Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 16:25:30 +0100 Subject: [PATCH 17/62] SQL formatting --- sql_scripts/flow_sheet_values.sql | 1 - sql_scripts/get_hospital_visit_id.sql | 2 -- sql_scripts/lab_results.sql | 1 - sql_scripts/lab_test_names.sql | 2 +- 4 files changed, 1 insertion(+), 5 deletions(-) diff --git a/sql_scripts/flow_sheet_values.sql b/sql_scripts/flow_sheet_values.sql index af9b76f..3be8624 100644 --- a/sql_scripts/flow_sheet_values.sql +++ b/sql_scripts/flow_sheet_values.sql @@ -32,4 +32,3 @@ AND vo.valid_from BETWEEN %(yesterday)s AND %(today)s AND vo.hospital_visit_id = %(hospital_visit_id)s GROUP BY DateTimeRecorded, Units, vo.comment - diff --git a/sql_scripts/get_hospital_visit_id.sql b/sql_scripts/get_hospital_visit_id.sql index 408447f..862a67f 100644 --- a/sql_scripts/get_hospital_visit_id.sql +++ b/sql_scripts/get_hospital_visit_id.sql @@ -3,5 +3,3 @@ select hospital_visit_id from star.hospital_visit as hv where hv.encounter = %(csn)s -- note the CSN must be in quotes - - diff --git a/sql_scripts/lab_results.sql b/sql_scripts/lab_results.sql index aa05264..81783ce 100644 --- a/sql_scripts/lab_results.sql +++ b/sql_scripts/lab_results.sql @@ -21,4 +21,3 @@ on r.lab_order_id = o.lab_order_id where o.hospital_visit_id = 'xx' and r.result_status like 'FINAL' and r.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') - diff --git a/sql_scripts/lab_test_names.sql b/sql_scripts/lab_test_names.sql index 85ce715..1838b72 100644 --- a/sql_scripts/lab_test_names.sql +++ b/sql_scripts/lab_test_names.sql @@ -2,4 +2,4 @@ select lab_test_definition_id as id, name, standardised_vocabulary as vocab from star.lab_test_definition as ltd -where ltd.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') \ No newline at end of file +where ltd.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') From 2b28e42a305f87153cd82a187db7d2446cc15302 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 16:44:13 +0100 Subject: [PATCH 18/62] Test with ehr lookup --- tests/test_snakemake_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 3bf8a4d..843ca1c 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -268,7 +268,7 @@ def _run_snakemake(tmp_path): tmp_exporter_env_path = tmp_path / "config/exporter.env" tmp_exporter_env_path.parent.mkdir(exist_ok=True) tmp_exporter_env_path.write_text( - "SNAKEMAKE_RULE_UNTIL=all_daily_hash_lookups\n" + "SNAKEMAKE_RULE_UNTIL=all_ehr_lookups\n" "SNAKEMAKE_CORES=1\n" "INSTANCE_NAME=pytest\n" "CSV_AGE_THRESHOLD_MINUTES=5\n" From d4f21212ff78fbda36fb5455e76aefdd9b0e0d2d Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 16:55:20 +0100 Subject: [PATCH 19/62] use caboodle testing --- tests/test_snakemake_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 843ca1c..908fed5 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -274,6 +274,7 @@ def _run_snakemake(tmp_path): "CSV_AGE_THRESHOLD_MINUTES=5\n" "ONLY_USE_CSV_FROM_YESTERDAY=False\n" "PROCESS_CSV_FROM_DATE=\n" + "CABOODLE_TESTING=TRUE\n" ) # run system under test (exporter container) in foreground compose_args = [ From a704072dfd9b8e2777cce06d59db0141c7ed36cc Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Tue, 18 Aug 2026 17:18:10 +0100 Subject: [PATCH 20/62] Trying to test ehr and hash lookup with ftps --- src/pipeline/Snakefile | 6 ++++++ tests/test_snakemake_integration.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/pipeline/Snakefile b/src/pipeline/Snakefile index 71042d9..30eb607 100644 --- a/src/pipeline/Snakefile +++ b/src/pipeline/Snakefile @@ -83,6 +83,12 @@ rule all_ehr_lookups: input: ALL_EHR_LOOKUPS +# a rule combining ehr and hash look ups to enable testing without ftps upload +rule all_ehr_and_hash_lookups: + input: + ALL_EHR_LOOKUPS, + ALL_DAILY_HASH_LOOKUPS + def input_file_maker(wc): unhashed_csn = hash_to_csn[wc.hashed_csn] # when using input functions, snakemake doesn't do its normal templating, you have to do it, hence the f-string diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 908fed5..522bb76 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -268,7 +268,7 @@ def _run_snakemake(tmp_path): tmp_exporter_env_path = tmp_path / "config/exporter.env" tmp_exporter_env_path.parent.mkdir(exist_ok=True) tmp_exporter_env_path.write_text( - "SNAKEMAKE_RULE_UNTIL=all_ehr_lookups\n" + "SNAKEMAKE_RULE_UNTIL=all_ehr_and_hash_lookups\n" "SNAKEMAKE_CORES=1\n" "INSTANCE_NAME=pytest\n" "CSV_AGE_THRESHOLD_MINUTES=5\n" From 8bd10d2bcdc3f55bdfe0c498fa86f54e5ea5c42a Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 19 Aug 2026 14:42:36 +0100 Subject: [PATCH 21/62] Moved sql --- {sql_scripts => src/sql}/flow_sheet_values.sql | 0 {sql_scripts => src/sql}/get_hospital_visit_id.sql | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {sql_scripts => src/sql}/flow_sheet_values.sql (100%) rename {sql_scripts => src/sql}/get_hospital_visit_id.sql (100%) diff --git a/sql_scripts/flow_sheet_values.sql b/src/sql/flow_sheet_values.sql similarity index 100% rename from sql_scripts/flow_sheet_values.sql rename to src/sql/flow_sheet_values.sql diff --git a/sql_scripts/get_hospital_visit_id.sql b/src/sql/get_hospital_visit_id.sql similarity index 100% rename from sql_scripts/get_hospital_visit_id.sql rename to src/sql/get_hospital_visit_id.sql From 76c8d9ffe2868b94e64f7c4570794dc44d04834a Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 19 Aug 2026 15:43:45 +0100 Subject: [PATCH 22/62] refactored db to make adding ehr lookup implementation clearer --- src/controller.py | 7 ++-- src/db.py | 49 ++++++++++++++--------- src/sql/mrn_based_on_bed_and_datetime.sql | 8 ++-- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/controller.py b/src/controller.py index 3dc3099..956be16 100644 --- a/src/controller.py +++ b/src/controller.py @@ -43,7 +43,6 @@ def reject_message(ch, delivery_tag, requeue): class WaveformController: def __init__(self): self.emap_db = db.starDB() - self.emap_db.init_query() self.emap_db.connect() def waveform_callback(self, ch, method_frame, _header_frame, body): @@ -76,7 +75,9 @@ def waveform_callback(self, ch, method_frame, _header_frame, body): ) lookup_success = True try: - matched_mrn = self.emap_db.get_row(location_string, observation_time) + matched_mrn = self.emap_db.get_matched_mrn( + location_string, observation_time + ) except ValueError: lookup_success = False logger.error( @@ -86,6 +87,7 @@ def waveform_callback(self, ch, method_frame, _header_frame, body): exc_info=True, ) matched_mrn = ("unmatched_mrn", "unmatched_nhs", "unmatched_csn", False) + # matched_mrn = ("1234568", "12345678", "12345678", False) except ConnectionError: logger.error("Database error, will try again", exc_info=True) reject_message(ch, method_frame.delivery_tag, True) @@ -96,7 +98,6 @@ def waveform_callback(self, ch, method_frame, _header_frame, body): logger.info("Research opt-out is set for mrn %s, not writing.", mrn) reject_message(ch, method_frame.delivery_tag, False) return - if writer.write_frame( waveform_data, source_variable_id, diff --git a/src/db.py b/src/db.py index 04eb37a..f6d78f6 100644 --- a/src/db.py +++ b/src/db.py @@ -11,7 +11,7 @@ class starDB: - sql_query: str = "" + mrn_lookup_query: str = "" connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( settings.UDS_DBNAME, # type:ignore settings.UDS_USERNAME, # type:ignore @@ -21,27 +21,32 @@ class starDB: settings.UDS_CONNECT_TIMEOUT, # type:ignore settings.UDS_QUERY_TIMEOUT, # type:ignore ) - connection_pool: pool.ThreadedConnectionPool + connection_pool: pool.SimpleConnectionPool - def connect(self): + def connect(self) -> None: self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) - def init_query(self): + def _init_mrn_lookup_query(self) -> None: with open("src/sql/mrn_based_on_bed_and_datetime.sql", "r") as file: - self.sql_query = sql.SQL(file.read()) - self.sql_query = self.sql_query.format( + self.mrn_lookup_query = sql.SQL(file.read()) # type:ignore + + self.mrn_lookup_query = self.mrn_lookup_query.format( schema_name=sql.Identifier(settings.SCHEMA_NAME) ) - def get_row(self, location_string: str, observation_datetime: datetime): + def get_matched_mrn( + self, location_string: str, observation_datetime: datetime + ) -> pd.DataFrame: parameters = { "location_string": location_string, "observation_datetime": observation_datetime, } + if self.mrn_lookup_query == "": + self._init_mrn_lookup_query() try: with self.connection_pool.getconn() as db_connection: with db_connection.cursor() as curs: - curs.execute(self.sql_query, parameters) + curs.execute(self.mrn_lookup_query, parameters) rows = curs.fetchall() self.connection_pool.putconn(db_connection) except psycopg2.errors.OperationalError as e: @@ -55,6 +60,9 @@ def get_row(self, location_string: str, observation_datetime: datetime): return rows[0] + def get_hospital_visit_from_csn(self, csn: str) -> str: + return "not implemented yet" + class caboodleDB: """For querying the caboodle database to extract electronic healthcare records per @@ -69,10 +77,10 @@ class caboodleDB: settings.CABOODLE_CONNECT_TIMEOUT, # type:ignore settings.CABOODLE_QUERY_TIMEOUT, # type:ignore ) - connection_pool: pool.ThreadedConnectionPool + connection_pool: pool.SimpleConnectionPool fake_caboodle: bool - def connect(self): + def connect(self) -> None: """Set up connection to the database.""" self.fake_caboodle = True if settings.CABOODLE_TESTING == "TRUE" else False if not self.fake_caboodle: @@ -80,8 +88,19 @@ def connect(self): 1, 1, self.connection_string ) - def get_airflow(self, start_datetime: datetime, end_datetime: datetime, csn: str): + def get_airflow( + self, start_datetime: datetime, end_datetime: datetime, csn: str + ) -> pd.DataFrame: """Retrieve airflow data from database.""" + + with open("src/sql/airway.sql", "r") as file: + airway_query = sql.SQL(file.read()) + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "csn": csn, + } + if self.fake_caboodle: fake_airway = { "DateTimeRecorded": [0], @@ -90,13 +109,7 @@ def get_airflow(self, start_datetime: datetime, end_datetime: datetime, csn: str "TubeSize": [0], } return pd.DataFrame(data=fake_airway) - with open("src/sql/airway.sql", "r") as file: - airway_query = sql.SQL(file.read()) - parameters = { - "start_datetime": start_datetime, - "end_datetime": end_datetime, - "csn": csn, - } + return self._get_rows(airway_query, parameters) def _get_rows(self, sql_query: sql.SQL, parameters: dict): diff --git a/src/sql/mrn_based_on_bed_and_datetime.sql b/src/sql/mrn_based_on_bed_and_datetime.sql index 7eccf5e..4494d21 100644 --- a/src/sql/mrn_based_on_bed_and_datetime.sql +++ b/src/sql/mrn_based_on_bed_and_datetime.sql @@ -1,7 +1,7 @@ -/* Find a medical record number (MRN), NHS number, and contact serial number (CSN) based on location -string and date time. Returns a list of MRN, NHS numbers, and CSN with the -first entry being the most recent. -*/ +-- Find a medical record number (MRN), NHS number, and contact serial number (CSN) based on location +-- string and date time. Returns a list of MRN, NHS numbers, and CSN with the +-- first entry being the most recent. +-- SELECT mn.mrn as mrn, mn.nhs_number as nhs_number, From b7e5ce37f7fc6d7ba35aa32e3b89bae0ca6318e9 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 19 Aug 2026 16:22:42 +0100 Subject: [PATCH 23/62] Started on logic to get flowsheets, first need hospital visit number --- src/db.py | 33 +++++++++++++++++++-------- src/electronic_health_records/ehr.py | 34 +++++++++++++++------------- src/sql/get_hospital_visit_id.sql | 5 ++-- 3 files changed, 43 insertions(+), 29 deletions(-) diff --git a/src/db.py b/src/db.py index f6d78f6..13d8a3c 100644 --- a/src/db.py +++ b/src/db.py @@ -43,15 +43,8 @@ def get_matched_mrn( } if self.mrn_lookup_query == "": self._init_mrn_lookup_query() - try: - with self.connection_pool.getconn() as db_connection: - with db_connection.cursor() as curs: - curs.execute(self.mrn_lookup_query, parameters) - rows = curs.fetchall() - self.connection_pool.putconn(db_connection) - except psycopg2.errors.OperationalError as e: - self.connection_pool.putconn(db_connection) - raise ConnectionError(f"Data base error: {e}") + + rows = self._get_rows(self.mrn_lookup_query, parameters) # type: ignore if len(rows) != 1: raise ValueError( @@ -61,7 +54,27 @@ def get_matched_mrn( return rows[0] def get_hospital_visit_from_csn(self, csn: str) -> str: - return "not implemented yet" + with open("src/sql/get_hospital_visit_id.sql", "r") as file: + hv_query = sql.SQL(file.read()) + + parameters = { + "schema": settings.SCHEMA_NAME, + "csn": csn, + } + + return self._get_rows(hv_query, parameters) + + def _get_rows(self, sql_query: sql.SQL, parameters: dict): + try: + with self.connection_pool.getconn() as db_connection: + with db_connection.cursor() as curs: + curs.execute(sql_query, parameters) + rows = curs.fetchall() + self.connection_pool.putconn(db_connection) + except psycopg2.errors.OperationalError as e: + self.connection_pool.putconn(db_connection) + raise ConnectionError(f"Data base error: {e}") + return rows class caboodleDB: diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index 48fc930..7470282 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -2,7 +2,7 @@ from datetime import datetime, timedelta -from db import caboodleDB +from db import caboodleDB, starDB from csv_writer import write_ehr from pseudon.pseudon import pseudonymise_relevant_columns @@ -18,28 +18,24 @@ def ehr_for_csv(date_str: str, original_csn: str, hashed_csn: str) -> None: :param hashed_csn: the pseudonymised hash to use for file output. """ - db_connection = caboodleDB() - db_connection.connect() + caboodle_connection = caboodleDB() + caboodle_connection.connect() - _ehr_for_csv(date_str, original_csn, hashed_csn, db_connection) + star_connection = starDB() + star_connection.connect() + + _ehr_for_csv( + date_str, original_csn, hashed_csn, caboodle_connection, star_connection + ) def _ehr_for_csv( date_str: str, original_csn: str, hashed_csn: str, - db_connection: caboodleDB, + caboodle_connection: caboodleDB, + star_connection: starDB, ) -> None: - """Extracts electronic healthcare records for a given csn and writes the results to - a pseudonymised csv file for a single day. - - This is a privacy-sensitive area of code. Unhashed CSNs must not appear in uploaded - files. - :param date_str: the date to look up data for - :param original_csn: the csn to base look up on. - :param hashed_csn: the pseudonymised hash to use for file output. - :param db_connection: connection to the caboodle database. - """ # will pick up the logger config defined in the snakemake job (ie. log to file) logger = logging.getLogger(__name__) @@ -47,7 +43,13 @@ def _ehr_for_csv( start_datetime = datetime.strptime(date_str, "%Y-%m-%d") end_datetime = start_datetime + timedelta(days=1) - airflow = db_connection.get_airflow(start_datetime, end_datetime, original_csn) + airflow = caboodle_connection.get_airflow( + start_datetime, end_datetime, original_csn + ) + + hospital_visit = star_connection.get_hospital_visit_from_csn(original_csn) + + logger.info(hospital_visit) safe_columns = [ "DateTimeRecorded", diff --git a/src/sql/get_hospital_visit_id.sql b/src/sql/get_hospital_visit_id.sql index 862a67f..15af7a0 100644 --- a/src/sql/get_hospital_visit_id.sql +++ b/src/sql/get_hospital_visit_id.sql @@ -1,5 +1,4 @@ - +-- Retrieve the hospital_visit_id associated with the csn value applied to this function -- - -select hospital_visit_id from star.hospital_visit as hv +select hospital_visit_id from %(schema)s.hospital_visit as hv where hv.encounter = %(csn)s -- note the CSN must be in quotes From 87a8ea1c086205a072ddd445d558f9865b7931ef Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 19 Aug 2026 16:47:14 +0100 Subject: [PATCH 24/62] The exporter needs to access uds, do duplicated some of the config from controller --- config.EXAMPLE/exporter.env.EXAMPLE | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/config.EXAMPLE/exporter.env.EXAMPLE b/config.EXAMPLE/exporter.env.EXAMPLE index c49d503..ed8efeb 100644 --- a/config.EXAMPLE/exporter.env.EXAMPLE +++ b/config.EXAMPLE/exporter.env.EXAMPLE @@ -46,3 +46,14 @@ CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds # a testing flag for Caboodle. If set TRUE caboodle connection will # fail silently and ehr file will be created with fake data CABOODLE_TESTING="FALSE" + +# The following is duplicated from controller.env +# the exporter needs access to uds +UDS_DBNAME="fakeuds" +UDS_USERNAME="inform_user" +UDS_PASSWORD="inform" +UDS_HOST="172.17.0.1" +UDS_PORT="5433" +UDS_CONNECT_TIMEOUT="10" +UDS_QUERY_TIMEOUT="3000" +SCHEMA_NAME="schemaname" From 8219da8d5d105d23377a8ddfbdaa6bf58818f03f Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 20 Aug 2026 08:22:42 +0100 Subject: [PATCH 25/62] Use enviroment to manage sql locations as it varies between containers --- config.EXAMPLE/controller.env.EXAMPLE | 2 ++ config.EXAMPLE/exporter.env.EXAMPLE | 2 ++ src/db.py | 6 +++--- src/settings.py | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/config.EXAMPLE/controller.env.EXAMPLE b/config.EXAMPLE/controller.env.EXAMPLE index e055216..66d9672 100644 --- a/config.EXAMPLE/controller.env.EXAMPLE +++ b/config.EXAMPLE/controller.env.EXAMPLE @@ -13,3 +13,5 @@ RABBITMQ_PASSWORD="my_pw" RABBITMQ_HOST="localhost" RABBITMQ_PORT=5672 RABBITMQ_QUEUE="waveform" + +SQL_PATH="./src/sql/" diff --git a/config.EXAMPLE/exporter.env.EXAMPLE b/config.EXAMPLE/exporter.env.EXAMPLE index ed8efeb..e8ed0f9 100644 --- a/config.EXAMPLE/exporter.env.EXAMPLE +++ b/config.EXAMPLE/exporter.env.EXAMPLE @@ -57,3 +57,5 @@ UDS_PORT="5433" UDS_CONNECT_TIMEOUT="10" UDS_QUERY_TIMEOUT="3000" SCHEMA_NAME="schemaname" + +SQL_PATH="/app/src/sql/" diff --git a/src/db.py b/src/db.py index 13d8a3c..1d62798 100644 --- a/src/db.py +++ b/src/db.py @@ -27,7 +27,7 @@ def connect(self) -> None: self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) def _init_mrn_lookup_query(self) -> None: - with open("src/sql/mrn_based_on_bed_and_datetime.sql", "r") as file: + with open(settings.SQL_PATH + "mrn_based_on_bed_and_datetime.sql", "r") as file: self.mrn_lookup_query = sql.SQL(file.read()) # type:ignore self.mrn_lookup_query = self.mrn_lookup_query.format( @@ -54,7 +54,7 @@ def get_matched_mrn( return rows[0] def get_hospital_visit_from_csn(self, csn: str) -> str: - with open("src/sql/get_hospital_visit_id.sql", "r") as file: + with open(settings.SQL_PATH + "get_hospital_visit_id.sql", "r") as file: hv_query = sql.SQL(file.read()) parameters = { @@ -106,7 +106,7 @@ def get_airflow( ) -> pd.DataFrame: """Retrieve airflow data from database.""" - with open("src/sql/airway.sql", "r") as file: + with open(settings.SQL_PATH + "airway.sql", "r") as file: airway_query = sql.SQL(file.read()) parameters = { "start_datetime": start_datetime, diff --git a/src/settings.py b/src/settings.py index 0309839..df0322c 100644 --- a/src/settings.py +++ b/src/settings.py @@ -49,3 +49,4 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("LOG_LEVEL", default_value="INFO") get_from_env("INSTANCE_NAME", required=True) +get_from_env("SQL_PATH") From 76144af140f1478b81502ad75dc6ed26ef8629d9 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 20 Aug 2026 08:24:50 +0100 Subject: [PATCH 26/62] path into test --- tests/test_snakemake_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 522bb76..68a023c 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -275,6 +275,7 @@ def _run_snakemake(tmp_path): "ONLY_USE_CSV_FROM_YESTERDAY=False\n" "PROCESS_CSV_FROM_DATE=\n" "CABOODLE_TESTING=TRUE\n" + "SQL_PATH=/app/src/sql/\n" ) # run system under test (exporter container) in foreground compose_args = [ From 093563f2710a731d47f17b61992291eedca702d5 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 20 Aug 2026 08:32:59 +0100 Subject: [PATCH 27/62] Use format rather than parameters for schema name --- src/db.py | 3 ++- src/sql/get_hospital_visit_id.sql | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/db.py b/src/db.py index 1d62798..d826231 100644 --- a/src/db.py +++ b/src/db.py @@ -57,8 +57,9 @@ def get_hospital_visit_from_csn(self, csn: str) -> str: with open(settings.SQL_PATH + "get_hospital_visit_id.sql", "r") as file: hv_query = sql.SQL(file.read()) + hv_query = hv_query.format(schema_name=sql.Identifier(settings.SCHEMA_NAME)) # type: ignore + parameters = { - "schema": settings.SCHEMA_NAME, "csn": csn, } diff --git a/src/sql/get_hospital_visit_id.sql b/src/sql/get_hospital_visit_id.sql index 15af7a0..8cd2f79 100644 --- a/src/sql/get_hospital_visit_id.sql +++ b/src/sql/get_hospital_visit_id.sql @@ -1,4 +1,4 @@ -- Retrieve the hospital_visit_id associated with the csn value applied to this function -- -select hospital_visit_id from %(schema)s.hospital_visit as hv +select hospital_visit_id from {schema_name}.hospital_visit as hv where hv.encounter = %(csn)s -- note the CSN must be in quotes From ece41749a0fd8f12893e085389d5b9878a2cd641 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 20 Aug 2026 09:00:32 +0100 Subject: [PATCH 28/62] Default for sql path to simplify testing --- src/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings.py b/src/settings.py index df0322c..151306b 100644 --- a/src/settings.py +++ b/src/settings.py @@ -49,4 +49,4 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("LOG_LEVEL", default_value="INFO") get_from_env("INSTANCE_NAME", required=True) -get_from_env("SQL_PATH") +get_from_env("SQL_PATH", default_value="./src/sql/") From 6d3f4700840de0790d340550f7218027e7cf46e3 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 20 Aug 2026 09:14:56 +0100 Subject: [PATCH 29/62] Updated mocked function name in test controller --- src/settings.py | 2 +- tests/test_controller.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/settings.py b/src/settings.py index 151306b..df0322c 100644 --- a/src/settings.py +++ b/src/settings.py @@ -49,4 +49,4 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("LOG_LEVEL", default_value="INFO") get_from_env("INSTANCE_NAME", required=True) -get_from_env("SQL_PATH", default_value="./src/sql/") +get_from_env("SQL_PATH") diff --git a/tests/test_controller.py b/tests/test_controller.py index d4fd611..3bee1e2 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -22,9 +22,11 @@ def test_controller_callback(monkeypatch, opt_out, db_connect_failure, bad_data): emap_db_mock = Mock() if db_connect_failure: - emap_db_mock.get_row.side_effect = ConnectionError("mock database error") + emap_db_mock.get_matched_mrn.side_effect = ConnectionError( + "mock database error" + ) else: - emap_db_mock.get_row.return_value = ("mrn", "nhsno", "csn", opt_out) + emap_db_mock.get_matched_mrn.return_value = ("mrn", "nhsno", "csn", opt_out) monkeypatch.setattr("controller.db.starDB", Mock(return_value=emap_db_mock)) write_frame_mock = Mock(return_value=True) @@ -56,12 +58,12 @@ def test_controller_callback(monkeypatch, opt_out, db_connect_failure, bad_data) if not bad_data: # we at least tried to query the DB - emap_db_mock.get_row.assert_called_once() + emap_db_mock.get_matched_mrn.assert_called_once() if bad_data: write_frame_mock.assert_not_called() # db should not even have been queried if data was bad - emap_db_mock.get_row.assert_not_called() + emap_db_mock.get_matched_mrn.assert_not_called() channel_mock.basic_reject.assert_called_once_with(delivery_tag, False) channel_mock.basic_ack.assert_not_called() elif db_connect_failure: From d6b51e8fe1214264f7d2a5aa6068043b76137af9 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 24 Aug 2026 09:07:45 +0100 Subject: [PATCH 30/62] Enable a fake star db shortcut so that snakemake integration test can run --- src/db.py | 12 +++++++++++- src/settings.py | 1 + tests/test_snakemake_integration.py | 1 + 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/db.py b/src/db.py index d826231..1d2459b 100644 --- a/src/db.py +++ b/src/db.py @@ -22,8 +22,16 @@ class starDB: settings.UDS_QUERY_TIMEOUT, # type:ignore ) connection_pool: pool.SimpleConnectionPool + fake_star: bool = False def connect(self) -> None: + self.fake_star = True if settings.STARDB_TESTING == "TRUE" else False + if not self.fake_star: + self.connection_pool = pool.SimpleConnectionPool( + 1, 1, self.connection_string + ) + + self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) def _init_mrn_lookup_query(self) -> None: @@ -62,6 +70,8 @@ def get_hospital_visit_from_csn(self, csn: str) -> str: parameters = { "csn": csn, } + if self.fake_star: + return '12345678' return self._get_rows(hv_query, parameters) @@ -92,7 +102,7 @@ class caboodleDB: settings.CABOODLE_QUERY_TIMEOUT, # type:ignore ) connection_pool: pool.SimpleConnectionPool - fake_caboodle: bool + fake_caboodle: bool = False def connect(self) -> None: """Set up connection to the database.""" diff --git a/src/settings.py b/src/settings.py index df0322c..c70c05a 100644 --- a/src/settings.py +++ b/src/settings.py @@ -22,6 +22,7 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("UDS_PORT") get_from_env("UDS_CONNECT_TIMEOUT") get_from_env("UDS_QUERY_TIMEOUT") +get_from_env("STARDB_TESTING") get_from_env("SCHEMA_NAME") get_from_env("RABBITMQ_USERNAME") get_from_env("RABBITMQ_PASSWORD") diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 68a023c..aa00818 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -274,6 +274,7 @@ def _run_snakemake(tmp_path): "CSV_AGE_THRESHOLD_MINUTES=5\n" "ONLY_USE_CSV_FROM_YESTERDAY=False\n" "PROCESS_CSV_FROM_DATE=\n" + "STARDB_TESTING=TRUE\n" "CABOODLE_TESTING=TRUE\n" "SQL_PATH=/app/src/sql/\n" ) From f4a629b1fa6b844879b39e4ef95abb6f971e45d2 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Mon, 24 Aug 2026 09:37:10 +0100 Subject: [PATCH 31/62] Don't connect for fake star --- src/db.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/db.py b/src/db.py index 1d2459b..59cdcae 100644 --- a/src/db.py +++ b/src/db.py @@ -31,9 +31,6 @@ def connect(self) -> None: 1, 1, self.connection_string ) - - self.connection_pool = pool.SimpleConnectionPool(1, 1, self.connection_string) - def _init_mrn_lookup_query(self) -> None: with open(settings.SQL_PATH + "mrn_based_on_bed_and_datetime.sql", "r") as file: self.mrn_lookup_query = sql.SQL(file.read()) # type:ignore @@ -71,7 +68,7 @@ def get_hospital_visit_from_csn(self, csn: str) -> str: "csn": csn, } if self.fake_star: - return '12345678' + return "12345678" return self._get_rows(hv_query, parameters) From df0db43f9c708b01ff1f45eacf8e573b3f55845a Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 26 Aug 2026 08:59:09 +0100 Subject: [PATCH 32/62] Tidied up getting for hospital visit number --- src/db.py | 8 +++++++- src/settings.py | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/db.py b/src/db.py index 59cdcae..25c8e0d 100644 --- a/src/db.py +++ b/src/db.py @@ -70,7 +70,13 @@ def get_hospital_visit_from_csn(self, csn: str) -> str: if self.fake_star: return "12345678" - return self._get_rows(hv_query, parameters) + hospital_visit_id = self._get_rows(hv_query, parameters) + + # fetchall returns a list of tuples. We want the first element of the first tuple + if not isinstance(hospital_visit_id[0][0], str): + raise TypeError(f"hospital_visit_id is not string {hospital_visit_id}") + + return hospital_visit_id[0][0] def _get_rows(self, sql_query: sql.SQL, parameters: dict): try: diff --git a/src/settings.py b/src/settings.py index c70c05a..8f6cc91 100644 --- a/src/settings.py +++ b/src/settings.py @@ -50,4 +50,4 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("LOG_LEVEL", default_value="INFO") get_from_env("INSTANCE_NAME", required=True) -get_from_env("SQL_PATH") +get_from_env("SQL_PATH", default_value="./src/sql/") From e84f031966de6a5a7249f04ef0eb8651f95714e1 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 26 Aug 2026 11:12:40 +0100 Subject: [PATCH 33/62] return integer hospital visit id --- src/db.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/db.py b/src/db.py index 25c8e0d..acbd855 100644 --- a/src/db.py +++ b/src/db.py @@ -58,7 +58,7 @@ def get_matched_mrn( return rows[0] - def get_hospital_visit_from_csn(self, csn: str) -> str: + def get_hospital_visit_from_csn(self, csn: str) -> int: with open(settings.SQL_PATH + "get_hospital_visit_id.sql", "r") as file: hv_query = sql.SQL(file.read()) @@ -68,13 +68,15 @@ def get_hospital_visit_from_csn(self, csn: str) -> str: "csn": csn, } if self.fake_star: - return "12345678" + return 12345678 hospital_visit_id = self._get_rows(hv_query, parameters) # fetchall returns a list of tuples. We want the first element of the first tuple - if not isinstance(hospital_visit_id[0][0], str): - raise TypeError(f"hospital_visit_id is not string {hospital_visit_id}") + if not isinstance(hospital_visit_id[0][0], int): + logger.warning( + f"hospital_visit_id[0][0] is not integer {hospital_visit_id}" + ) return hospital_visit_id[0][0] From ca4789a2ba4b73cea0a45614d5dbf66791a37fe8 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Wed, 26 Aug 2026 15:02:11 +0100 Subject: [PATCH 34/62] Added flowsheet query to ehr --- src/db.py | 24 ++++++++++++++++++++++++ src/electronic_health_records/ehr.py | 20 +++++++++++++++----- src/sql/flow_sheet_values.sql | 2 +- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/db.py b/src/db.py index acbd855..abaf3dd 100644 --- a/src/db.py +++ b/src/db.py @@ -141,6 +141,30 @@ def get_airflow( return self._get_rows(airway_query, parameters) + def get_flowsheets( + self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int + ) -> pd.DataFrame: + """Retrieve airflow data from database.""" + + with open(settings.SQL_PATH + "flow_sheet_values.sql", "r") as file: + flowsheet_query = sql.SQL(file.read()) + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "hospital_visit_id": hospital_visit_id, + } + + if self.fake_caboodle: + fake_flowsheet = { + "DateTimeRecorded": [0], + "Temperature": [0], + "Noradrenaline": [0], + "Metaraminol": [0], + } + return pd.DataFrame(data=fake_flowsheet) + + return self._get_rows(flowsheet_query, parameters) + def _get_rows(self, sql_query: sql.SQL, parameters: dict): try: with self.connection_pool.getconn() as db_connection: diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index 7470282..eefc830 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -1,6 +1,7 @@ import logging from datetime import datetime, timedelta +import pandas as pd from db import caboodleDB, starDB from csv_writer import write_ehr @@ -47,22 +48,31 @@ def _ehr_for_csv( start_datetime, end_datetime, original_csn ) - hospital_visit = star_connection.get_hospital_visit_from_csn(original_csn) + hospital_visit_id = star_connection.get_hospital_visit_from_csn(original_csn) - logger.info(hospital_visit) + logger.info(hospital_visit_id) + + flowsheet_values = caboodle_connection.get_flowsheets( + start_datetime, end_datetime, hospital_visit_id + ) + + ehr_data = pd.concat([airflow, flowsheet_values]) safe_columns = [ "DateTimeRecorded", "PlacementInstant", "RemovalInstant", "TubeSize", + "Temperature", + "Noradrenaline", + "Metaraminol", ] - airflow = pseudonymise_relevant_columns(airflow, safe_columns) + ehr_data = pseudonymise_relevant_columns(ehr_data, safe_columns) - write_ehr(airflow, date_str, hashed_csn) + write_ehr(ehr_data, date_str, hashed_csn) - logger.info(airflow) + logger.info(ehr_data) # delete csn once we no longer need it del original_csn diff --git a/src/sql/flow_sheet_values.sql b/src/sql/flow_sheet_values.sql index 3be8624..b66b626 100644 --- a/src/sql/flow_sheet_values.sql +++ b/src/sql/flow_sheet_values.sql @@ -28,7 +28,7 @@ LEFT JOIN star.visit_observation_type AS vt ON vo.visit_observation_type_id = vt.visit_observation_type_id WHERE vt.id_in_application IN ('6', '3040102622', '12946') -AND vo.valid_from BETWEEN %(yesterday)s AND %(today)s +AND vo.valid_from BETWEEN %(start_datetime)s AND %(end_datetime)s AND vo.hospital_visit_id = %(hospital_visit_id)s GROUP BY DateTimeRecorded, Units, vo.comment From cba9b54c1ab8018b209827d0f0cc3e5bd64cead2 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 27 Aug 2026 14:47:37 +0100 Subject: [PATCH 35/62] moved sql and implemented lab results --- src/db.py | 29 ++++++++++++++++++++++++ src/electronic_health_records/ehr.py | 27 ++++++++++++++++------ {sql_scripts => src/sql}/lab_results.sql | 6 ++--- 3 files changed, 52 insertions(+), 10 deletions(-) rename {sql_scripts => src/sql}/lab_results.sql (88%) diff --git a/src/db.py b/src/db.py index abaf3dd..f468881 100644 --- a/src/db.py +++ b/src/db.py @@ -165,6 +165,35 @@ def get_flowsheets( return self._get_rows(flowsheet_query, parameters) + def get_lab_results( + self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int + ) -> pd.DataFrame: + """Retrieve lab result data from caboodle.""" + + with open(settings.SQL_PATH + "lab_results.sql", "r") as file: + flowsheet_query = sql.SQL(file.read()) + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "hospital_visit_id": hospital_visit_id, + } + + if self.fake_caboodle: + fake_flowsheet = { + "DateTimeRecorded": [0], + "Units": ["None"], + "Abnormal_result": ["No"], + "Comments": ["None"], + "C-reactive protein 1": ["-"], + "CSF WCC TUBE 1": ["-"], + "CSF WCC TUBE 2": ["-"], + "CSF WCC TUBE 3": ["-"], + "C-reactive protein 2": ["-"], + } + return pd.DataFrame(data=fake_flowsheet) + + return self._get_rows(flowsheet_query, parameters) + def _get_rows(self, sql_query: sql.SQL, parameters: dict): try: with self.connection_pool.getconn() as db_connection: diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index eefc830..0f6ebc8 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -44,20 +44,27 @@ def _ehr_for_csv( start_datetime = datetime.strptime(date_str, "%Y-%m-%d") end_datetime = start_datetime + timedelta(days=1) - airflow = caboodle_connection.get_airflow( - start_datetime, end_datetime, original_csn - ) + # we need hospital visit id for flowsheet and lab_result queries hospital_visit_id = star_connection.get_hospital_visit_from_csn(original_csn) - logger.info(hospital_visit_id) + # fetch data from caboodle + airflow = caboodle_connection.get_airflow( + start_datetime, end_datetime, original_csn + ) flowsheet_values = caboodle_connection.get_flowsheets( start_datetime, end_datetime, hospital_visit_id ) - ehr_data = pd.concat([airflow, flowsheet_values]) + lab_results = caboodle_connection.get_lab_results( + start_datetime, end_datetime, hospital_visit_id + ) + ehr_data = pd.concat([airflow, flowsheet_values, lab_results]) + + # we can pseudonymise to safe, although at the moment all columns + # are considered safe safe_columns = [ "DateTimeRecorded", "PlacementInstant", @@ -66,13 +73,19 @@ def _ehr_for_csv( "Temperature", "Noradrenaline", "Metaraminol", + "Units", + "Abnormal_result", + "C-reactive protein", + "CSF WCC TUBE 1", + "CSF WCC TUBE 2", + "CSF WCC TUBE 3", + "C-reactive protein", + "Comments", # Free text comments could contain sensitive information. Should we hash it? ] ehr_data = pseudonymise_relevant_columns(ehr_data, safe_columns) write_ehr(ehr_data, date_str, hashed_csn) - logger.info(ehr_data) - # delete csn once we no longer need it del original_csn diff --git a/sql_scripts/lab_results.sql b/src/sql/lab_results.sql similarity index 88% rename from sql_scripts/lab_results.sql rename to src/sql/lab_results.sql index 21b8b5d..eb2d0cb 100644 --- a/sql_scripts/lab_results.sql +++ b/src/sql/lab_results.sql @@ -8,11 +8,11 @@ SELECT r.result_last_modified_datetime AS DateTimeRecorded, - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '1001') AS "C-reactive protein", + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '1001') AS "C-reactive protein 1", MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793054') AS "CSF WCC TUBE 1", MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793057') AS "CSF WCC TUBE 2", MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793060') AS "CSF WCC TUBE 3", - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '722790196') AS "C-reactive protein" + MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '722790196') AS "C-reactive protein 2" r.units AS Units, r.abnormal_flag AS Abnormal_result, @@ -29,7 +29,7 @@ r.lab_test_definition_id IN ('1001', '390793057', '390793060', '722790196') -AND vo.valid_from BETWEEN %(yesterday)s AND %(today)s + AND vo.valid_from BETWEEN %(start_datetime)s AND %(end_datetime)s AND o.hospital_visit_id = %(hospital_visit_id)s GROUP BY DateTimeRecorded, Units, Abnormal_result, Comments From c0850e53c2337f966ef0f219ee1d1d2fa66edda5 Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 27 Aug 2026 16:34:12 +0100 Subject: [PATCH 36/62] Fixed column names in pseudon --- src/electronic_health_records/ehr.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index 0f6ebc8..51e1dde 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -75,11 +75,11 @@ def _ehr_for_csv( "Metaraminol", "Units", "Abnormal_result", - "C-reactive protein", + "C-reactive protein 1", "CSF WCC TUBE 1", "CSF WCC TUBE 2", "CSF WCC TUBE 3", - "C-reactive protein", + "C-reactive protein 2", "Comments", # Free text comments could contain sensitive information. Should we hash it? ] From f497fc4164ae9667fac15c7731106266fae9769b Mon Sep 17 00:00:00 2001 From: Stephen Thompson Date: Thu, 27 Aug 2026 16:53:58 +0100 Subject: [PATCH 37/62] Flowsheets and labresults are star, not caboodle --- src/db.py | 116 +++++++++++++++------------ src/electronic_health_records/ehr.py | 4 +- src/sql/flow_sheet_values.sql | 4 +- src/sql/lab_results.sql | 4 +- 4 files changed, 69 insertions(+), 59 deletions(-) diff --git a/src/db.py b/src/db.py index f468881..7f199b3 100644 --- a/src/db.py +++ b/src/db.py @@ -80,6 +80,69 @@ def get_hospital_visit_from_csn(self, csn: str) -> int: return hospital_visit_id[0][0] + def get_flowsheets( + self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int + ) -> pd.DataFrame: + """Retrieve airflow data from database.""" + + with open(settings.SQL_PATH + "flow_sheet_values.sql", "r") as file: + flowsheet_query = sql.SQL(file.read()) + + flowsheet_query = flowsheet_query.format( + schema_name=sql.Identifier(settings.SCHEMA_NAME) + ) # type: ignore + + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "hospital_visit_id": hospital_visit_id, + } + + if self.fake_star: + fake_flowsheet = { + "DateTimeRecorded": [0], + "Temperature": [0], + "Noradrenaline": [0], + "Metaraminol": [0], + } + return pd.DataFrame(data=fake_flowsheet) + + return self._get_rows(flowsheet_query, parameters) + + def get_lab_results( + self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int + ) -> pd.DataFrame: + """Retrieve lab result data from caboodle.""" + + with open(settings.SQL_PATH + "lab_results.sql", "r") as file: + lab_result_query = sql.SQL(file.read()) + + lab_result_query = lab_result_query.format( + schema_name=sql.Identifier(settings.SCHEMA_NAME) + ) # type: ignore + + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "hospital_visit_id": hospital_visit_id, + } + + if self.fake_star: + fake_lab_result = { + "DateTimeRecorded": [0], + "Units": ["None"], + "Abnormal_result": ["No"], + "Comments": ["None"], + "C-reactive protein 1": ["-"], + "CSF WCC TUBE 1": ["-"], + "CSF WCC TUBE 2": ["-"], + "CSF WCC TUBE 3": ["-"], + "C-reactive protein 2": ["-"], + } + return pd.DataFrame(data=fake_lab_result) + + return self._get_rows(lab_result_query, parameters) + def _get_rows(self, sql_query: sql.SQL, parameters: dict): try: with self.connection_pool.getconn() as db_connection: @@ -141,59 +204,6 @@ def get_airflow( return self._get_rows(airway_query, parameters) - def get_flowsheets( - self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int - ) -> pd.DataFrame: - """Retrieve airflow data from database.""" - - with open(settings.SQL_PATH + "flow_sheet_values.sql", "r") as file: - flowsheet_query = sql.SQL(file.read()) - parameters = { - "start_datetime": start_datetime, - "end_datetime": end_datetime, - "hospital_visit_id": hospital_visit_id, - } - - if self.fake_caboodle: - fake_flowsheet = { - "DateTimeRecorded": [0], - "Temperature": [0], - "Noradrenaline": [0], - "Metaraminol": [0], - } - return pd.DataFrame(data=fake_flowsheet) - - return self._get_rows(flowsheet_query, parameters) - - def get_lab_results( - self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int - ) -> pd.DataFrame: - """Retrieve lab result data from caboodle.""" - - with open(settings.SQL_PATH + "lab_results.sql", "r") as file: - flowsheet_query = sql.SQL(file.read()) - parameters = { - "start_datetime": start_datetime, - "end_datetime": end_datetime, - "hospital_visit_id": hospital_visit_id, - } - - if self.fake_caboodle: - fake_flowsheet = { - "DateTimeRecorded": [0], - "Units": ["None"], - "Abnormal_result": ["No"], - "Comments": ["None"], - "C-reactive protein 1": ["-"], - "CSF WCC TUBE 1": ["-"], - "CSF WCC TUBE 2": ["-"], - "CSF WCC TUBE 3": ["-"], - "C-reactive protein 2": ["-"], - } - return pd.DataFrame(data=fake_flowsheet) - - return self._get_rows(flowsheet_query, parameters) - def _get_rows(self, sql_query: sql.SQL, parameters: dict): try: with self.connection_pool.getconn() as db_connection: diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index 51e1dde..5cd5ef4 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -53,11 +53,11 @@ def _ehr_for_csv( start_datetime, end_datetime, original_csn ) - flowsheet_values = caboodle_connection.get_flowsheets( + flowsheet_values = star_connection.get_flowsheets( start_datetime, end_datetime, hospital_visit_id ) - lab_results = caboodle_connection.get_lab_results( + lab_results = star_connection.get_lab_results( start_datetime, end_datetime, hospital_visit_id ) diff --git a/src/sql/flow_sheet_values.sql b/src/sql/flow_sheet_values.sql index b66b626..9cb4288 100644 --- a/src/sql/flow_sheet_values.sql +++ b/src/sql/flow_sheet_values.sql @@ -22,9 +22,9 @@ SELECT vo.unit AS Units, vo.comment AS Comments -FROM star.visit_observation AS vo +FROM {schema_name}.visit_observation AS vo -LEFT JOIN star.visit_observation_type AS vt +LEFT JOIN {schema_name}.visit_observation_type AS vt ON vo.visit_observation_type_id = vt.visit_observation_type_id WHERE vt.id_in_application IN ('6', '3040102622', '12946') diff --git a/src/sql/lab_results.sql b/src/sql/lab_results.sql index eb2d0cb..426f48d 100644 --- a/src/sql/lab_results.sql +++ b/src/sql/lab_results.sql @@ -18,8 +18,8 @@ SELECT r.abnormal_flag AS Abnormal_result, r.comment AS Comments -FROM star.lab_result AS r -LEFT JOIN star.lab_order AS o +FROM {schema_name}.lab_result AS r +LEFT JOIN {schema_name}.lab_order AS o ON r.lab_order_id = o.lab_order_id WHERE r.result_status like 'FINAL' From 8a083379ad1623e8c1bff0381b1d347888fac9ff Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Mon, 31 Aug 2026 12:28:05 +0100 Subject: [PATCH 38/62] Auto stash before merge of "sk/queries" and "st/queries-to-snakemake" --- sql_scripts/lab_test_names.sql | 10 ++++++++++ src/sql/flow_sheet_values.sql | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/sql_scripts/lab_test_names.sql b/sql_scripts/lab_test_names.sql index 1838b72..cbd6070 100644 --- a/sql_scripts/lab_test_names.sql +++ b/sql_scripts/lab_test_names.sql @@ -3,3 +3,13 @@ select lab_test_definition_id as id, standardised_vocabulary as vocab from star.lab_test_definition as ltd where ltd.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') + +id name vocab +1001 C-reactive protein +390793054 CSF WCC TUBE 1 +390793057 CSF WCC TUBE 2 +390793060 CSF WCC TUBE 3 +722790196 C-reactive protein + +PaCO2 39947 - in star +PaO2 40191 \ No newline at end of file diff --git a/src/sql/flow_sheet_values.sql b/src/sql/flow_sheet_values.sql index 9cb4288..f035b1b 100644 --- a/src/sql/flow_sheet_values.sql +++ b/src/sql/flow_sheet_values.sql @@ -31,4 +31,4 @@ WHERE vt.id_in_application IN ('6', '3040102622', '12946') AND vo.valid_from BETWEEN %(start_datetime)s AND %(end_datetime)s AND vo.hospital_visit_id = %(hospital_visit_id)s -GROUP BY DateTimeRecorded, Units, vo.comment +GROUP BY DateTimeRecorded, Units, Comments From b93c17dbeb46247e7e0dabb8ad3fd65664041a13 Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Mon, 7 Sep 2026 10:15:39 +0100 Subject: [PATCH 39/62] star flow sheet query --- src/sql/flow_sheet_values.sql | 48 ++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/sql/flow_sheet_values.sql b/src/sql/flow_sheet_values.sql index f035b1b..7076491 100644 --- a/src/sql/flow_sheet_values.sql +++ b/src/sql/flow_sheet_values.sql @@ -1,34 +1,46 @@ ---get the flow sheet values for the particular visit on a particular day --- the flow sheet numbers are recorded as id_in_application in the visit_observation_type table +-- get the flow sheet values for the particular visit on a particular day +-- the flow sheet numbers are recorded as id_in_application +-- in the visit_observation_type table -- Temperature 6 --- Noradrenalin 3040102622 +-- Noradrenaline 3040102622 -- Metaraminol 12946 +-- PaO2 40191 +-- PaCO2 39947 + SELECT - vo.observation_datetime AS DateTimeRecorded, + vo.observation_datetime AS "DateTimeRecorded", - (array_agg(vo.value_as_real) FILTER ( + MAX(vo.value_as_real) FILTER ( WHERE vt.id_in_application = '6' - ))[1] AS "Temperature", + ) AS "Temperature", - (array_agg(vo.value_as_real) FILTER ( + MAX(vo.value_as_real) FILTER ( WHERE vt.id_in_application = '3040102622' - ))[1] AS "Noradrenaline", + ) AS "Noradrenaline", - (array_agg(vo.value_as_real) FILTER ( + MAX(vo.value_as_real) FILTER ( WHERE vt.id_in_application = '12946' - ))[1] AS "Metaraminol", - - vo.unit AS Units, - vo.comment AS Comments - + ) AS "Metaraminol", + + MAX(vo.value_as_real) FILTER ( + WHERE vt.id_in_application = '40191' + ) AS "PaO2", + + MAX(vo.value_as_real) FILTER ( + WHERE vt.id_in_application = '39947' + ) AS "PaCO2", + + vo.unit AS "Units" + FROM {schema_name}.visit_observation AS vo LEFT JOIN {schema_name}.visit_observation_type AS vt ON vo.visit_observation_type_id = vt.visit_observation_type_id -WHERE vt.id_in_application IN ('6', '3040102622', '12946') -AND vo.valid_from BETWEEN %(start_datetime)s AND %(end_datetime)s -AND vo.hospital_visit_id = %(hospital_visit_id)s +WHERE + vt.id_in_application IN ('6', '3040102622', '12946', '40191', '39947') + AND vo.valid_from >= %(start_datetime)s AND vo.valid_from < %(end$_datetime)s + AND vo.hospital_visit_id = %(hospital_visit_id)s -GROUP BY DateTimeRecorded, Units, Comments +GROUP BY "DateTimeRecorded", "Units" From 2fc22407b73ae6454e8e12fd11f556134d65535a Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Mon, 7 Sep 2026 10:37:47 +0100 Subject: [PATCH 40/62] finished star lab queries --- src/sql/lab_results.sql | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/sql/lab_results.sql b/src/sql/lab_results.sql index 426f48d..e013af7 100644 --- a/src/sql/lab_results.sql +++ b/src/sql/lab_results.sql @@ -1,35 +1,27 @@ -- This selects the values of lab tests --- 1011 CRP --- 722790196 CRP --- 390793054 WCC --- 390793057 WCC --- 390793060 WCC +-- 1011 C REACTIVE PROTEIN +-- 686 WHITE CELL COUNT SELECT - r.result_last_modified_datetime AS DateTimeRecorded, + r.result_last_modified_datetime AS "DateTimeRecorded", - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '1001') AS "C-reactive protein 1", - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793054') AS "CSF WCC TUBE 1", - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793057') AS "CSF WCC TUBE 2", - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '390793060') AS "CSF WCC TUBE 3", - MAX(r.value_as_real) FILTER (WHERE r.lab_test_definition_id = '722790196') AS "C-reactive protein 2" + MAX(r.value_as_real) FILTER + (WHERE r.lab_test_definition_id = '1001') AS "CRP", - r.units AS Units, - r.abnormal_flag AS Abnormal_result, - r.comment AS Comments + MAX(r.value_as_real) FILTER + (WHERE r.lab_test_definition_id = '686') AS "WCC", + + r.units AS "Units" FROM {schema_name}.lab_result AS r LEFT JOIN {schema_name}.lab_order AS o ON r.lab_order_id = o.lab_order_id -WHERE r.result_status like 'FINAL' -AND -r.lab_test_definition_id IN ('1001', - '390793054', - '390793057', - '390793060', - '722790196') - AND vo.valid_from BETWEEN %(start_datetime)s AND %(end_datetime)s -AND o.hospital_visit_id = %(hospital_visit_id)s +WHERE + r.result_status LIKE 'FINAL' + AND r.lab_test_definition_id IN ('1001', '686') + AND r.result_last_modified_datetime >= %(start_datetime)s + AND r.result_last_modified_datetime < %(end_datetime)s + AND o.hospital_visit_id = %(hospital_visit_id)s -GROUP BY DateTimeRecorded, Units, Abnormal_result, Comments +GROUP BY "DateTimeRecorded", "Units" From f9461496a028a05716b4a1e3ed1f5aa947b2a119 Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Tue, 8 Sep 2026 15:34:04 +0100 Subject: [PATCH 41/62] removed un new scripts and moved read me to current script location --- sql_scripts/lab_test_names.sql | 15 --------------- {sql_scripts => src/sql}/README.md | 30 +++++++++++++++++++++--------- 2 files changed, 21 insertions(+), 24 deletions(-) delete mode 100644 sql_scripts/lab_test_names.sql rename {sql_scripts => src/sql}/README.md (58%) diff --git a/sql_scripts/lab_test_names.sql b/sql_scripts/lab_test_names.sql deleted file mode 100644 index cbd6070..0000000 --- a/sql_scripts/lab_test_names.sql +++ /dev/null @@ -1,15 +0,0 @@ -select lab_test_definition_id as id, - name, - standardised_vocabulary as vocab -from star.lab_test_definition as ltd -where ltd.lab_test_definition_id in ('1001', '390793054', '390793057', '390793060', '722790196') - -id name vocab -1001 C-reactive protein -390793054 CSF WCC TUBE 1 -390793057 CSF WCC TUBE 2 -390793060 CSF WCC TUBE 3 -722790196 C-reactive protein - -PaCO2 39947 - in star -PaO2 40191 \ No newline at end of file diff --git a/sql_scripts/README.md b/src/sql/README.md similarity index 58% rename from sql_scripts/README.md rename to src/sql/README.md index 98ae5af..38e248c 100644 --- a/sql_scripts/README.md +++ b/src/sql/README.md @@ -1,5 +1,16 @@ # Notes on putting together the EHR needed +## Private scripts + +This is a public repository and so we cannot include any scripts that are proprietary from the hospital system EPIC. +These are included in a separate private repository named waveform-private-queries. This has a directory structure + +[top-level]/src/sql + +so that it can be copied directly onto the directory structure of this repository and thus all scripts will be contained in the same place upon deployment. + + + ## Goal The ultimate aim is to have one csv per patient per day which looks roughly like @@ -19,15 +30,16 @@ The ultimate aim is to have one csv per patient per day which looks roughly like | script | arguments | record | location of script in repo | database | |- | --- | --- |- | --- | | mrn_based_on_bed_and_datetime.sql | location string | csn |waveform-controller/src/sql | star | -| get_hospital_visit_id.sql| csn | hospital_visit_id | waveform-controller/sql_scripts| star | -| flow_sheet_values.sql| hospital_visit_id/today/yesterday | part of table above | waveform-controller/sql_scripts| star | -| airway.sql | csn/today/yesterday | part of the table above | waveform-controller/sql_scripts | caboodle | -| sputum_secretions.sql | csn/today/yesterday | part of the table above | waveform-controller/sql_scripts | caboodle | +| get_hospital_visit_id.sql| csn | hospital_visit_id | waveform-controller/src/sql| star | +| flow_sheet_values.sql| hospital_visit_id/today/yesterday | part of table above | waveform-controller/src/sql| star | +| lab_results.sql | csn/today/yesterday | part of the table above | waveform-controller/src/sql | star | +| sputum_secretions.sql | csn/today/yesterday | part of the table above | waveform-private-queries/src/sql | caboodle | +| reposition.sql | csn/today/yesterday | part of the table above | waveform-private-queries/src/sql | caboodle | +--- ## Unfinished scripts -lab_results.sql need dealing with in the same way as flow_sheet_values - -lab_test_names.sql forms part of the above query but is useful for exploring - -We need scripts for any of the items in the a tracker that have not yet been covered. +| script | arguments | record | location of script in repo | database | +|- | --- | --- |- | --- | +| airway.sql | csn/today/yesterday | part of the table above | waveform-private-queries/src/sql | caboodle | +--- From e249346823239541fb1942b9db40b222dab3321f Mon Sep 17 00:00:00 2001 From: Sarah Keating Date: Tue, 8 Sep 2026 15:40:40 +0100 Subject: [PATCH 42/62] added note about getting scripts from the waveform-private-queries repository as part of deployment --- docs/deployment.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/deployment.md b/docs/deployment.md index 1c8940a..3d78a81 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -35,6 +35,7 @@ sledgehammer approach which is rather similar to * Delete all Emap tables in `star_dev` as per Emap deployment instructions. * Waveform: `docker compose down` to bring everything down * git pull and rebuild containers for the two repos. +* **REMEMBER: that the files from waveform-private-queries repository need to be copied to the waveform-controller/src/sql directory.** * Change config if necessary * Bring it all up again From 1218cb597c269829363061d3f71f049e736b2339 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Wed, 9 Sep 2026 16:43:16 +0100 Subject: [PATCH 43/62] Post-merge linting fix --- src/csv_writer.py | 2 +- src/pseudon/pseudon.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/csv_writer.py b/src/csv_writer.py index 1b4cb5e..c5529f4 100644 --- a/src/csv_writer.py +++ b/src/csv_writer.py @@ -116,6 +116,6 @@ def write_ehr( filename = WAVEFORM_PSEUDONYMISED_EHR / f"{stem}_ehr.csv" filename.parent.mkdir(exist_ok=True, parents=True) - df.to_csv(filename, index = False) + df.to_csv(filename, index=False) return True diff --git a/src/pseudon/pseudon.py b/src/pseudon/pseudon.py index 1a859d5..41af2a2 100644 --- a/src/pseudon/pseudon.py +++ b/src/pseudon/pseudon.py @@ -215,7 +215,7 @@ def add_waveform_metadata_to_table( return existing_table -def pseudonymise_relevant_columns(df: pd.DataFrame, safe_columns: [str]): +def pseudonymise_relevant_columns(df: pd.DataFrame, safe_columns: list[str]): """ "csn", "mrn", "location" are examples of columns that must be pseudonymised. However, it's safer to list which columns *don't* need to be pseudonymised. Eg. you From 6de2a63c36a814726058519f517bdd6c47f26d5d Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Wed, 9 Sep 2026 17:47:07 +0100 Subject: [PATCH 44/62] SCHEMA_NAME is accessed regardless of testing mode, so it has to exist --- tests/test_snakemake_integration.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index d28eacb..9e719fa 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -378,6 +378,7 @@ def _run_snakemake(tmp_path): "STARDB_TESTING=TRUE\n" "CABOODLE_TESTING=TRUE\n" "SQL_PATH=/app/src/sql/\n" + "SCHEMA_NAME=\n" # in testing mode, value doesn't matter but it has to exist ) # Collect coverage from Python processes inside the exporter container From 8c4bf8285a242e5a2fb73b4def776432999da487 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Thu, 10 Sep 2026 16:14:22 +0100 Subject: [PATCH 45/62] Create a space for scripts that can't be in this repo to be installed to --- src/db.py | 2 +- src/sql/private/.gitignore | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 src/sql/private/.gitignore diff --git a/src/db.py b/src/db.py index 7f199b3..48c28f6 100644 --- a/src/db.py +++ b/src/db.py @@ -185,7 +185,7 @@ def get_airflow( ) -> pd.DataFrame: """Retrieve airflow data from database.""" - with open(settings.SQL_PATH + "airway.sql", "r") as file: + with open(settings.SQL_PATH + "private/airway.sql", "r") as file: airway_query = sql.SQL(file.read()) parameters = { "start_datetime": start_datetime, diff --git a/src/sql/private/.gitignore b/src/sql/private/.gitignore new file mode 100644 index 0000000..083a98e --- /dev/null +++ b/src/sql/private/.gitignore @@ -0,0 +1,2 @@ +# for queries that come from another source that are not to be included in this repo +* From 7968f1779952d3fa65c86e9e56436e6e0407b8e7 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Thu, 10 Sep 2026 18:03:04 +0100 Subject: [PATCH 46/62] Bring in private SQL scripts --- .github/workflows/pytest.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index 670b270..c1c017a 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -14,6 +14,13 @@ jobs: PROJECT_ROOT: ${{ github.workspace }}/waveform-controller environment: hasher steps: + - name: Generate GitHub App Token + id: generate-token + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ secrets.SAFEHR_READ_ONLY_WORKER_ID }} + private-key: ${{ secrets.SAFEHR_READ_ONLY_WORKER_KEY }} + repositories: waveform-private-queries - uses: actions/checkout@v5 with: path: waveform-controller @@ -23,6 +30,13 @@ jobs: repository: SAFEHR-data/PIXL ref: e29f4b15c3b9d21c9a6e08c272aca7773311b32c # pragma: allowlist secret path: PIXL + - name: Checkout private SQL scripts + uses: actions/checkout@v5 + with: + repository: SAFEHR-data/waveform-private-queries + token: ${{ steps.generate-token.outputs.token }} + ref: 0345bb6e3ed51b05e46982434911c9138c84a035 + path: waveform-private-queries - name: Install uv uses: astral-sh/setup-uv@v7 @@ -65,6 +79,8 @@ jobs: echo "AZURE_KEY_VAULT_SECRET_NAME=${AZURE_KEY_VAULT_SECRET_NAME}" } >> ../config/hasher.env + # install private SQL scripts + cp ../waveform-private-queries/src/sql/*.sql src/sql/private/ # exporter config can't be done here because test_snakemake_integration.py wires in its own config file - name: Run the tests From 85ea6ada0b86d29f9bb266ce3af88e4d12853bb1 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Fri, 11 Sep 2026 16:20:00 +0100 Subject: [PATCH 47/62] Linting fix --- .github/workflows/pytest.yml | 2 +- tests/test_snakemake_integration.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml index c1c017a..be4479b 100644 --- a/.github/workflows/pytest.yml +++ b/.github/workflows/pytest.yml @@ -35,7 +35,7 @@ jobs: with: repository: SAFEHR-data/waveform-private-queries token: ${{ steps.generate-token.outputs.token }} - ref: 0345bb6e3ed51b05e46982434911c9138c84a035 + ref: 0345bb6e3ed51b05e46982434911c9138c84a035 # pragma: allowlist secret path: waveform-private-queries - name: Install uv diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index 9e719fa..bba7102 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -378,7 +378,7 @@ def _run_snakemake(tmp_path): "STARDB_TESTING=TRUE\n" "CABOODLE_TESTING=TRUE\n" "SQL_PATH=/app/src/sql/\n" - "SCHEMA_NAME=\n" # in testing mode, value doesn't matter but it has to exist + "SCHEMA_NAME=\n" # in testing mode, value doesn't matter but it has to exist ) # Collect coverage from Python processes inside the exporter container From 1e4466841007c2964b2e8dff5cace0854092d820 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Fri, 11 Sep 2026 16:17:36 +0100 Subject: [PATCH 48/62] Add private repo to setup instructions --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 833acb7..26badb4 100644 --- a/README.md +++ b/README.md @@ -64,10 +64,14 @@ separate to the Emap project root. ##### Clone repos -Clone this repo (`waveform-controller`) and [PIXL](https://github.com/SAFEHR-data/PIXL), +Clone +* this repo (`waveform-controller`) +* [PIXL](https://github.com/SAFEHR-data/PIXL), +* [Private query repo](https://github.com/SAFEHR-data/waveform-private-queries) both inside your root directory. -Inside the PIXL repo, checkout the commit that we have pinned in [workflow file](.github/workflows/pytest.yml). +Inside the PIXL and Private query repos, checkout the respective commits that +we have pinned in [the workflow file](.github/workflows/pytest.yml). If on a system that has access to sensitive data, disable push remotes on all cloned repos as follows: ``` From 5b3222ab87c591074ed0e14b5e2168ba0e302be7 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 11:57:39 +0100 Subject: [PATCH 49/62] fixup docs for private sql scripts --- docs/deployment.md | 2 +- src/sql/README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index aaac4a5..4d9b496 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -35,7 +35,7 @@ sledgehammer approach which is rather similar to * Delete all Emap tables in `star_dev` as per Emap deployment instructions. * Waveform: `docker compose down` to bring everything down * git pull and rebuild containers for the two repos. -* **REMEMBER: that the files from waveform-private-queries repository need to be copied to the waveform-controller/src/sql directory.** +* **REMEMBER: that the files from waveform-private-queries repository need to be copied to the waveform-controller/src/sql/private directory.** * Change config if necessary * Bring it all up again diff --git a/src/sql/README.md b/src/sql/README.md index 38e248c..799fab3 100644 --- a/src/sql/README.md +++ b/src/sql/README.md @@ -5,7 +5,7 @@ This is a public repository and so we cannot include any scripts that are proprietary from the hospital system EPIC. These are included in a separate private repository named waveform-private-queries. This has a directory structure -[top-level]/src/sql +[top-level]/src/sql/private so that it can be copied directly onto the directory structure of this repository and thus all scripts will be contained in the same place upon deployment. From d990007e60f5eee67774a95b4bc09fb9b37198e0 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Fri, 11 Sep 2026 19:58:03 +0100 Subject: [PATCH 50/62] Match changed signature in PIXL ftp method --- src/exporter/ftps.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/exporter/ftps.py b/src/exporter/ftps.py index 54ac620..71bc946 100644 --- a/src/exporter/ftps.py +++ b/src/exporter/ftps.py @@ -6,7 +6,11 @@ from time import perf_counter from typing import Any -from core.uploader._ftps import _connect_to_ftp, _create_and_set_as_cwd_multi_path +from core.uploader._ftps import ( + _connect_to_ftp, + _create_and_set_as_cwd_multi_path, + ImplicitFtpTls, +) import settings import telemetry @@ -103,6 +107,7 @@ def do_upload_multiple( settings.FTPS_PORT, settings.FTPS_USERNAME, settings.FTPS_PASSWORD, + ImplicitFtpTls, ) _create_and_set_as_cwd_multi_path(ftp, remote_project_dir) command = f"STOR {remote_tar_filename}" From 1dac661b3c39669fb28c549e1a505f4b004cda4d Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 15:58:10 +0100 Subject: [PATCH 51/62] Use importlib.resources to locate SQL files and thus remove need for settings.SQL_PATH. Refactor SQL query loading code. Increase log dumping in integration test. --- .dockerignore | 1 + config.EXAMPLE/controller.env.EXAMPLE | 1 - config.EXAMPLE/exporter.env.EXAMPLE | 1 - pyproject.toml | 4 ++ src/db.py | 63 +++++++++++++-------------- src/settings.py | 1 - tests/test_snakemake_integration.py | 15 +++---- 7 files changed, 42 insertions(+), 44 deletions(-) diff --git a/.dockerignore b/.dockerignore index 2a0c6b6..e740dab 100644 --- a/.dockerignore +++ b/.dockerignore @@ -4,3 +4,4 @@ .git **/.mypy_cache **/.pytest_tmp +**/build diff --git a/config.EXAMPLE/controller.env.EXAMPLE b/config.EXAMPLE/controller.env.EXAMPLE index 301c353..123a3b0 100644 --- a/config.EXAMPLE/controller.env.EXAMPLE +++ b/config.EXAMPLE/controller.env.EXAMPLE @@ -14,7 +14,6 @@ RABBITMQ_HOST="localhost" RABBITMQ_PORT=5672 RABBITMQ_QUEUE="waveform" -SQL_PATH="./src/sql/" # OpenTelemetry OTLP/HTTP endpoint of the LGTM collector. OTEL_EXPORTER_OTLP_ENDPOINT="http://lgtm:4318" OTEL_SERVICE_NAME=waveform-controller diff --git a/config.EXAMPLE/exporter.env.EXAMPLE b/config.EXAMPLE/exporter.env.EXAMPLE index 299e9fe..46aea11 100644 --- a/config.EXAMPLE/exporter.env.EXAMPLE +++ b/config.EXAMPLE/exporter.env.EXAMPLE @@ -58,7 +58,6 @@ UDS_CONNECT_TIMEOUT="10" UDS_QUERY_TIMEOUT="3000" SCHEMA_NAME="schemaname" -SQL_PATH="/app/src/sql/" # OpenTelemetry OTLP/HTTP endpoint of the LGTM collector. OTEL_EXPORTER_OTLP_ENDPOINT="http://lgtm:4318" OTEL_SERVICE_NAME=waveform-exporter diff --git a/pyproject.toml b/pyproject.toml index 6c15d11..0f9a094 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,6 +36,10 @@ coverage = [ [project.scripts] emap-extract-waveform = "controller:receiver" +# Include .sql files as part of the package (otherwise only editable installs would work) +[tool.setuptools.package-data] +sql = ["*.sql", "private/*.sql"] + [tool.pytest.ini_options] # Force temp dirs under the repo so Docker can mount them on macOS. # The default under /private/var/folders seems to silently fail (gives you an empty directory) diff --git a/src/db.py b/src/db.py index 48c28f6..e6446dc 100644 --- a/src/db.py +++ b/src/db.py @@ -1,8 +1,12 @@ from datetime import datetime +from typing import Optional + import pandas as pd import psycopg2 from psycopg2 import sql, pool import logging +from importlib import resources + import settings as settings # type:ignore @@ -10,8 +14,17 @@ logger = logging.getLogger(__name__) +def get_sql_query_with_schema( + query_rel_path: str, schema_name: Optional[str] = None +) -> sql.Composable: + query_text_tmpl = sql.SQL((resources.files("sql") / query_rel_path).read_text()) + if schema_name is None: + return query_text_tmpl + else: + return query_text_tmpl.format(schema_name=sql.Identifier(schema_name)) + + class starDB: - mrn_lookup_query: str = "" connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( settings.UDS_DBNAME, # type:ignore settings.UDS_USERNAME, # type:ignore @@ -31,14 +44,6 @@ def connect(self) -> None: 1, 1, self.connection_string ) - def _init_mrn_lookup_query(self) -> None: - with open(settings.SQL_PATH + "mrn_based_on_bed_and_datetime.sql", "r") as file: - self.mrn_lookup_query = sql.SQL(file.read()) # type:ignore - - self.mrn_lookup_query = self.mrn_lookup_query.format( - schema_name=sql.Identifier(settings.SCHEMA_NAME) - ) - def get_matched_mrn( self, location_string: str, observation_datetime: datetime ) -> pd.DataFrame: @@ -46,10 +51,11 @@ def get_matched_mrn( "location_string": location_string, "observation_datetime": observation_datetime, } - if self.mrn_lookup_query == "": - self._init_mrn_lookup_query() + mrn_lookup_query = get_sql_query_with_schema( + "mrn_based_on_bed_and_datetime.sql", settings.SCHEMA_NAME + ) - rows = self._get_rows(self.mrn_lookup_query, parameters) # type: ignore + rows = self._get_rows(mrn_lookup_query, parameters) # type: ignore if len(rows) != 1: raise ValueError( @@ -59,10 +65,9 @@ def get_matched_mrn( return rows[0] def get_hospital_visit_from_csn(self, csn: str) -> int: - with open(settings.SQL_PATH + "get_hospital_visit_id.sql", "r") as file: - hv_query = sql.SQL(file.read()) - - hv_query = hv_query.format(schema_name=sql.Identifier(settings.SCHEMA_NAME)) # type: ignore + hv_query = get_sql_query_with_schema( + "get_hospital_visit_id.sql", settings.SCHEMA_NAME + ) parameters = { "csn": csn, @@ -85,12 +90,9 @@ def get_flowsheets( ) -> pd.DataFrame: """Retrieve airflow data from database.""" - with open(settings.SQL_PATH + "flow_sheet_values.sql", "r") as file: - flowsheet_query = sql.SQL(file.read()) - - flowsheet_query = flowsheet_query.format( - schema_name=sql.Identifier(settings.SCHEMA_NAME) - ) # type: ignore + flowsheet_query = get_sql_query_with_schema( + "flow_sheet_values.sql", settings.SCHEMA_NAME + ) parameters = { "start_datetime": start_datetime, @@ -114,13 +116,9 @@ def get_lab_results( ) -> pd.DataFrame: """Retrieve lab result data from caboodle.""" - with open(settings.SQL_PATH + "lab_results.sql", "r") as file: - lab_result_query = sql.SQL(file.read()) - - lab_result_query = lab_result_query.format( - schema_name=sql.Identifier(settings.SCHEMA_NAME) - ) # type: ignore - + lab_result_query = get_sql_query_with_schema( + "lab_results.sql", settings.SCHEMA_NAME + ) parameters = { "start_datetime": start_datetime, "end_datetime": end_datetime, @@ -143,7 +141,7 @@ def get_lab_results( return self._get_rows(lab_result_query, parameters) - def _get_rows(self, sql_query: sql.SQL, parameters: dict): + def _get_rows(self, sql_query: sql.Composable, parameters: dict): try: with self.connection_pool.getconn() as db_connection: with db_connection.cursor() as curs: @@ -185,8 +183,7 @@ def get_airflow( ) -> pd.DataFrame: """Retrieve airflow data from database.""" - with open(settings.SQL_PATH + "private/airway.sql", "r") as file: - airway_query = sql.SQL(file.read()) + airway_query = get_sql_query_with_schema("private/airway.sql") parameters = { "start_datetime": start_datetime, "end_datetime": end_datetime, @@ -204,7 +201,7 @@ def get_airflow( return self._get_rows(airway_query, parameters) - def _get_rows(self, sql_query: sql.SQL, parameters: dict): + def _get_rows(self, sql_query: sql.Composable, parameters: dict): try: with self.connection_pool.getconn() as db_connection: with db_connection.cursor() as curs: diff --git a/src/settings.py b/src/settings.py index 08a7bfd..b93b367 100644 --- a/src/settings.py +++ b/src/settings.py @@ -50,7 +50,6 @@ def get_from_env(env_var, *, default_value=None, setting_name=None, required=Fal get_from_env("LOG_LEVEL", default_value="INFO") get_from_env("INSTANCE_NAME", required=True) -get_from_env("SQL_PATH", default_value="./src/sql/") # OpenTelemetry: OTLP/HTTP base URL of the LGTM collector, e.g. http://lgtm:4318 get_from_env("OTEL_EXPORTER_OTLP_ENDPOINT") diff --git a/tests/test_snakemake_integration.py b/tests/test_snakemake_integration.py index bba7102..3c53b7b 100644 --- a/tests/test_snakemake_integration.py +++ b/tests/test_snakemake_integration.py @@ -377,7 +377,6 @@ def _run_snakemake(tmp_path): "PROCESS_CSV_FROM_DATE=\n" "STARDB_TESTING=TRUE\n" "CABOODLE_TESTING=TRUE\n" - "SQL_PATH=/app/src/sql/\n" "SCHEMA_NAME=\n" # in testing mode, value doesn't matter but it has to exist ) @@ -412,14 +411,14 @@ def _run_snakemake(tmp_path): compose_args, cwd=REPO_ROOT, ) - # for convenience print the snakemake log files if they exist (on success or error) + # for debugging convenience print all the log files if they exist (on success or error) outer_logs_dir = tmp_path / "snakemake-logs" - outer_logs = sorted(outer_logs_dir.glob("snakemake-outer-log*.log")) - if not outer_logs: - print("No outer logs found") - for ol in outer_logs: - print(f"Log file {ol}:") - print(ol.read_text()) + all_logs = sorted(outer_logs_dir.rglob("*.log")) + if not all_logs: + print("No log files found") + for lf in all_logs: + print(f"Log file {lf}:") + print(lf.read_text()) # print all output then raise if there was an error print(f"stdout:\n{result.stdout}\n" f"stderr:\n{result.stderr}") result.check_returncode() From 44b1cf3e63cbf52a69e1990036f176a28055873c Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 18:39:53 +0100 Subject: [PATCH 52/62] Document how to do an ad hoc run --- docs/deployment.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 4d9b496..8773460 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -184,15 +184,14 @@ Here is the Filtering by variable is not currently possible. -# Run de-id on ad adhoc basis +# Run the Snakemake workflow on an ad hoc basis (ie. de-id, EHR lookup, upload) > [!NOTE] > Due to the way scheduled-script.sh pulls in its config from the config file, the contents of -> that file will override any env vars you specify on the command line below. +> that file will override any env vars you specify via docker below. So, temporarily +> changing the exporter.env config file is the only way to pass in a certain config. -You need to temporarily change the exporter.env config file to run this command. - -You are likely to want to set the following values (example date shown): +Variables you may wish to modify: ``` ONLY_USE_CSV_FROM_YESTERDAY=FALSE # something shorter than the standard 180 may be needed if you only just processed the data @@ -203,4 +202,4 @@ PROCESS_CSV_FROM_DATE=1234-12-12 docker compose run --entrypoint /app/exporter-scripts/scheduled-script.sh waveform-exporter ``` -Remember to put the config back afterwards. +Remember to revert the config changes you made if applicable. From 01636262887d8d7dc8cc25e83ddac39864a46e31 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 18:47:43 +0100 Subject: [PATCH 53/62] First go at split of db.py into PG and SQL Server versions --- src/controller.py | 4 +- src/db_mssql.py | 76 ++++++++++++++++++++++++++++ src/{db.py => db_pg.py} | 65 +----------------------- src/db_utils.py | 4 ++ src/electronic_health_records/ehr.py | 3 +- tests/test_controller.py | 2 +- 6 files changed, 87 insertions(+), 67 deletions(-) create mode 100644 src/db_mssql.py rename src/{db.py => db_pg.py} (68%) create mode 100644 src/db_utils.py diff --git a/src/controller.py b/src/controller.py index 3089177..8f9fd1a 100644 --- a/src/controller.py +++ b/src/controller.py @@ -12,7 +12,7 @@ from pika import spec from pika.adapters.blocking_connection import BlockingChannel -import db as db # type:ignore +import db_pg import settings as settings # type:ignore import csv_writer as writer # type:ignore import telemetry as telemetry # type:ignore @@ -107,7 +107,7 @@ def finalise_message(outcome: MessageOutcome): class WaveformController: def __init__(self): - self.emap_db = db.starDB() + self.emap_db = db_pg.starDB() self.emap_db.connect() def waveform_callback( diff --git a/src/db_mssql.py b/src/db_mssql.py new file mode 100644 index 0000000..83ddb9e --- /dev/null +++ b/src/db_mssql.py @@ -0,0 +1,76 @@ +from datetime import datetime +from typing import Optional + +import pandas as pd +import psycopg2 +from psycopg2 import sql, pool +import logging +from importlib import resources + + +import settings as settings # type:ignore +from db_utils import get_sql_query_text + +logging.basicConfig(format="%(levelname)s:%(asctime)s: %(message)s") +logger = logging.getLogger(__name__) + + +class caboodleDB: + """For querying the caboodle database to extract electronic healthcare records per + patient.""" + + connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( + settings.CABOODLE_DBNAME, # type:ignore + settings.CABOODLE_USERNAME, # type:ignore + settings.CABOODLE_PASSWORD, # type:ignore + settings.CABOODLE_HOST, # type:ignore + settings.CABOODLE_PORT, # type:ignore + settings.CABOODLE_CONNECT_TIMEOUT, # type:ignore + settings.CABOODLE_QUERY_TIMEOUT, # type:ignore + ) + connection_pool: pool.SimpleConnectionPool + fake_caboodle: bool = False + + def connect(self) -> None: + """Set up connection to the database.""" + self.fake_caboodle = True if settings.CABOODLE_TESTING == "TRUE" else False + if not self.fake_caboodle: + self.connection_pool = pool.SimpleConnectionPool( + 1, 1, self.connection_string + ) + + def get_airflow( + self, start_datetime: datetime, end_datetime: datetime, csn: str + ) -> pd.DataFrame: + """Retrieve airflow data from database.""" + + airway_query = get_sql_query_text("private/airway.sql") + parameters = { + "start_datetime": start_datetime, + "end_datetime": end_datetime, + "csn": csn, + } + + if self.fake_caboodle: + fake_airway = { + "DateTimeRecorded": [0], + "PlacementInstant": [0], + "RemovalInstant": [0], + "TubeSize": [0], + } + return pd.DataFrame(data=fake_airway) + + return self._get_rows(airway_query, parameters) + + def _get_rows(self, sql_query: sql.Composable, parameters: dict): + try: + with self.connection_pool.getconn() as db_connection: + with db_connection.cursor() as curs: + curs.execute(sql_query, parameters) + rows = curs.fetchall() + self.connection_pool.putconn(db_connection) + except psycopg2.errors.OperationalError as e: + self.connection_pool.putconn(db_connection) + raise ConnectionError(f"Data base error: {e}") + + return rows diff --git a/src/db.py b/src/db_pg.py similarity index 68% rename from src/db.py rename to src/db_pg.py index e6446dc..781a51a 100644 --- a/src/db.py +++ b/src/db_pg.py @@ -9,6 +9,7 @@ import settings as settings # type:ignore +from db_utils import get_sql_query_text logging.basicConfig(format="%(levelname)s:%(asctime)s: %(message)s") logger = logging.getLogger(__name__) @@ -17,13 +18,12 @@ def get_sql_query_with_schema( query_rel_path: str, schema_name: Optional[str] = None ) -> sql.Composable: - query_text_tmpl = sql.SQL((resources.files("sql") / query_rel_path).read_text()) + query_text_tmpl = sql.SQL(get_sql_query_text(query_rel_path)) if schema_name is None: return query_text_tmpl else: return query_text_tmpl.format(schema_name=sql.Identifier(schema_name)) - class starDB: connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( settings.UDS_DBNAME, # type:ignore @@ -152,64 +152,3 @@ def _get_rows(self, sql_query: sql.Composable, parameters: dict): self.connection_pool.putconn(db_connection) raise ConnectionError(f"Data base error: {e}") return rows - - -class caboodleDB: - """For querying the caboodle database to extract electronic healthcare records per - patient.""" - - connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( - settings.CABOODLE_DBNAME, # type:ignore - settings.CABOODLE_USERNAME, # type:ignore - settings.CABOODLE_PASSWORD, # type:ignore - settings.CABOODLE_HOST, # type:ignore - settings.CABOODLE_PORT, # type:ignore - settings.CABOODLE_CONNECT_TIMEOUT, # type:ignore - settings.CABOODLE_QUERY_TIMEOUT, # type:ignore - ) - connection_pool: pool.SimpleConnectionPool - fake_caboodle: bool = False - - def connect(self) -> None: - """Set up connection to the database.""" - self.fake_caboodle = True if settings.CABOODLE_TESTING == "TRUE" else False - if not self.fake_caboodle: - self.connection_pool = pool.SimpleConnectionPool( - 1, 1, self.connection_string - ) - - def get_airflow( - self, start_datetime: datetime, end_datetime: datetime, csn: str - ) -> pd.DataFrame: - """Retrieve airflow data from database.""" - - airway_query = get_sql_query_with_schema("private/airway.sql") - parameters = { - "start_datetime": start_datetime, - "end_datetime": end_datetime, - "csn": csn, - } - - if self.fake_caboodle: - fake_airway = { - "DateTimeRecorded": [0], - "PlacementInstant": [0], - "RemovalInstant": [0], - "TubeSize": [0], - } - return pd.DataFrame(data=fake_airway) - - return self._get_rows(airway_query, parameters) - - def _get_rows(self, sql_query: sql.Composable, parameters: dict): - try: - with self.connection_pool.getconn() as db_connection: - with db_connection.cursor() as curs: - curs.execute(sql_query, parameters) - rows = curs.fetchall() - self.connection_pool.putconn(db_connection) - except psycopg2.errors.OperationalError as e: - self.connection_pool.putconn(db_connection) - raise ConnectionError(f"Data base error: {e}") - - return rows diff --git a/src/db_utils.py b/src/db_utils.py new file mode 100644 index 0000000..82a40e2 --- /dev/null +++ b/src/db_utils.py @@ -0,0 +1,4 @@ +from importlib import resources + +def get_sql_query_text(query_rel_path: str) -> str: + return (resources.files("sql") / query_rel_path).read_text() diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index 5cd5ef4..5a44eac 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -3,7 +3,8 @@ from datetime import datetime, timedelta import pandas as pd -from db import caboodleDB, starDB +from db_mssql import caboodleDB +from db_pg import starDB from csv_writer import write_ehr from pseudon.pseudon import pseudonymise_relevant_columns diff --git a/tests/test_controller.py b/tests/test_controller.py index 9845288..bcf6eb6 100644 --- a/tests/test_controller.py +++ b/tests/test_controller.py @@ -176,7 +176,7 @@ def test_controller_callback( ) else: emap_db_mock.get_matched_mrn.return_value = ("mrn", "nhsno", "csn", opt_out) - monkeypatch.setattr("controller.db.starDB", Mock(return_value=emap_db_mock)) + monkeypatch.setattr("controller.db_pg.starDB", Mock(return_value=emap_db_mock)) write_frame_mock = Mock() monkeypatch.setattr("controller.writer.write_frame", write_frame_mock) From edc39a70172fde55056cae66ef83163092adcb3a Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 19:21:54 +0100 Subject: [PATCH 54/62] Cursor's first go at PG->MSSQL conversion --- Dockerfile | 6 ++- config.EXAMPLE/exporter.env.EXAMPLE | 2 +- pyproject.toml | 1 + src/db_mssql.py | 70 ++++++++++++++++------------- src/db_pg.py | 2 +- src/db_utils.py | 1 + src/sql/__init__.py | 0 tests/test_db_mssql.py | 63 ++++++++++++++++++++++++++ uv.lock | 43 +++++++++++++++++- 9 files changed, 154 insertions(+), 34 deletions(-) create mode 100644 src/sql/__init__.py create mode 100644 tests/test_db_mssql.py diff --git a/Dockerfile b/Dockerfile index dab631a..a3d80df 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,11 @@ LABEL authors="Stephen Thompson, Jeremy Stein" # put it on both images even though we only need it on exporter. RUN export DEBIAN_FRONTEND=noninteractive && \ apt-get update && \ - apt-get install --yes --no-install-recommends cron && \ + apt-get install --yes --no-install-recommends \ + cron \ + libgssapi-krb5-2 \ + libkrb5-3 \ + libltdl7 && \ apt-get autoremove --yes && apt-get clean --yes && rm -rf /var/lib/apt/lists/* # uv image label "0.12.5" COPY --from=ghcr.io/astral-sh/uv@sha256:e85be844203885286c60ffad8a858d48afb6c5a5c237ca0e67f12e74b8f174b1 /uv /uvx /bin/ diff --git a/config.EXAMPLE/exporter.env.EXAMPLE b/config.EXAMPLE/exporter.env.EXAMPLE index 46aea11..89b736c 100644 --- a/config.EXAMPLE/exporter.env.EXAMPLE +++ b/config.EXAMPLE/exporter.env.EXAMPLE @@ -38,7 +38,7 @@ CABOODLE_DBNAME="fakecab" CABOODLE_USERNAME="inform_user" CABOODLE_PASSWORD="inform" CABOODLE_HOST="localhost" -CABOODLE_PORT="5433" +CABOODLE_PORT="1433" CABOODLE_CONNECT_TIMEOUT="10" # in seconds CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds diff --git a/pyproject.toml b/pyproject.toml index 0f9a094..cc17338 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,6 +10,7 @@ dependencies = [ "pika>=1.3.2", "pre-commit>=4.5.0", "snakemake==9.14.5", + "mssql-python>=1.13.0", "opentelemetry-distro==0.63b1", "opentelemetry-exporter-otlp-proto-http==1.42.1", # need to be compatible with PIXL, which currently pins 2.9.11 (arguably it shouldn't) diff --git a/src/db_mssql.py b/src/db_mssql.py index 83ddb9e..e9299f2 100644 --- a/src/db_mssql.py +++ b/src/db_mssql.py @@ -1,42 +1,55 @@ from datetime import datetime -from typing import Optional +from math import ceil +import mssql_python import pandas as pd -import psycopg2 -from psycopg2 import sql, pool -import logging -from importlib import resources - import settings as settings # type:ignore from db_utils import get_sql_query_text -logging.basicConfig(format="%(levelname)s:%(asctime)s: %(message)s") -logger = logging.getLogger(__name__) + +def _odbc_escape(value: object) -> str: + """Quote a value for use in an ODBC connection string.""" + return "{" + str(value).replace("}", "}}") + "}" + + +def _get_connection_string() -> str: + return "Server={server};Database={database};UID={username};PWD={password};".format( + server=_odbc_escape( + f"{settings.CABOODLE_HOST},{settings.CABOODLE_PORT}" # type:ignore + ), + database=_odbc_escape(settings.CABOODLE_DBNAME), # type:ignore + username=_odbc_escape(settings.CABOODLE_USERNAME), # type:ignore + password=_odbc_escape(settings.CABOODLE_PASSWORD), # type:ignore + ) class caboodleDB: """For querying the caboodle database to extract electronic healthcare records per patient.""" - connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( - settings.CABOODLE_DBNAME, # type:ignore - settings.CABOODLE_USERNAME, # type:ignore - settings.CABOODLE_PASSWORD, # type:ignore - settings.CABOODLE_HOST, # type:ignore - settings.CABOODLE_PORT, # type:ignore - settings.CABOODLE_CONNECT_TIMEOUT, # type:ignore - settings.CABOODLE_QUERY_TIMEOUT, # type:ignore - ) - connection_pool: pool.SimpleConnectionPool + connection_string: str + db_connection: mssql_python.Connection fake_caboodle: bool = False def connect(self) -> None: """Set up connection to the database.""" - self.fake_caboodle = True if settings.CABOODLE_TESTING == "TRUE" else False + self.fake_caboodle = settings.CABOODLE_TESTING == "TRUE" if not self.fake_caboodle: - self.connection_pool = pool.SimpleConnectionPool( - 1, 1, self.connection_string + self.connection_string = _get_connection_string() + # CABOODLE_QUERY_TIMEOUT is configured in milliseconds, while + # mssql-python accepts whole seconds. + query_timeout_seconds = ceil( + int(settings.CABOODLE_QUERY_TIMEOUT) / 1000 # type:ignore + ) + self.db_connection = mssql_python.connect( + self.connection_string, + timeout=query_timeout_seconds, + attrs_before={ + mssql_python.SQL_ATTR_LOGIN_TIMEOUT: int( + settings.CABOODLE_CONNECT_TIMEOUT # type:ignore + ) + }, ) def get_airflow( @@ -62,15 +75,12 @@ def get_airflow( return self._get_rows(airway_query, parameters) - def _get_rows(self, sql_query: sql.Composable, parameters: dict): + def _get_rows(self, sql_query: str, parameters: dict): try: - with self.connection_pool.getconn() as db_connection: - with db_connection.cursor() as curs: - curs.execute(sql_query, parameters) - rows = curs.fetchall() - self.connection_pool.putconn(db_connection) - except psycopg2.errors.OperationalError as e: - self.connection_pool.putconn(db_connection) - raise ConnectionError(f"Data base error: {e}") + with self.db_connection.cursor() as curs: + curs.execute(sql_query, parameters) + rows = curs.fetchall() + except mssql_python.OperationalError as e: + raise ConnectionError(f"Database error: {e}") from e return rows diff --git a/src/db_pg.py b/src/db_pg.py index 781a51a..0dd85f1 100644 --- a/src/db_pg.py +++ b/src/db_pg.py @@ -5,7 +5,6 @@ import psycopg2 from psycopg2 import sql, pool import logging -from importlib import resources import settings as settings # type:ignore @@ -24,6 +23,7 @@ def get_sql_query_with_schema( else: return query_text_tmpl.format(schema_name=sql.Identifier(schema_name)) + class starDB: connection_string: str = "dbname={} user={} password={} host={} port={} connect_timeout={} options='-c statement_timeout={}'".format( settings.UDS_DBNAME, # type:ignore diff --git a/src/db_utils.py b/src/db_utils.py index 82a40e2..d005b06 100644 --- a/src/db_utils.py +++ b/src/db_utils.py @@ -1,4 +1,5 @@ from importlib import resources + def get_sql_query_text(query_rel_path: str) -> str: return (resources.files("sql") / query_rel_path).read_text() diff --git a/src/sql/__init__.py b/src/sql/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_db_mssql.py b/tests/test_db_mssql.py new file mode 100644 index 0000000..3928b3c --- /dev/null +++ b/tests/test_db_mssql.py @@ -0,0 +1,63 @@ +from unittest.mock import MagicMock + +import pytest + +import db_mssql + + +def test_connect_uses_mssql_connection_string_and_timeouts(monkeypatch): + monkeypatch.setattr(db_mssql.settings, "CABOODLE_TESTING", "FALSE") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_HOST", "sql.example.com") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_PORT", "1433") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_DBNAME", "Caboodle") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_USERNAME", "reader") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_PASSWORD", "p;ass}") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_CONNECT_TIMEOUT", "10") + monkeypatch.setattr(db_mssql.settings, "CABOODLE_QUERY_TIMEOUT", "3001") + connection = MagicMock() + connect = MagicMock(return_value=connection) + monkeypatch.setattr(db_mssql.mssql_python, "connect", connect) + + database = db_mssql.caboodleDB() + database.connect() + + connect.assert_called_once_with( + ( + "Server={sql.example.com,1433};Database={Caboodle};" + "UID={reader};PWD={p;ass}}};" + ), + timeout=4, + attrs_before={db_mssql.mssql_python.SQL_ATTR_LOGIN_TIMEOUT: 10}, + ) + assert database.db_connection is connection + + +def test_get_rows_executes_named_parameters(): + cursor = MagicMock() + cursor.fetchall.return_value = [("row",)] + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + database = db_mssql.caboodleDB() + database.db_connection = connection + parameters = {"csn": "123"} + + rows = database._get_rows("SELECT * FROM Encounter WHERE CSN = %(csn)s", parameters) + + cursor.execute.assert_called_once_with( + "SELECT * FROM Encounter WHERE CSN = %(csn)s", parameters + ) + assert rows == [("row",)] + + +def test_get_rows_translates_operational_errors(): + cursor = MagicMock() + cursor.execute.side_effect = db_mssql.mssql_python.OperationalError( + "timed out", "timed out" + ) + connection = MagicMock() + connection.cursor.return_value.__enter__.return_value = cursor + database = db_mssql.caboodleDB() + database.db_connection = connection + + with pytest.raises(ConnectionError, match="Database error"): + database._get_rows("SELECT 1", {}) diff --git a/uv.lock b/uv.lock index b063abf..92a3903 100644 --- a/uv.lock +++ b/uv.lock @@ -935,6 +935,45 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/5e/75/bd9b7bb966668920f06b200e84454c8f3566b102183bc55c5473d96cb2b9/msal_extensions-1.3.1-py3-none-any.whl", hash = "sha256:96d3de4d034504e969ac5e85bae8106c8373b5c6568e4c8fa7af2eca9dbe6bca", size = 20583, upload-time = "2025-03-14T23:51:03.016Z" }, ] +[[package]] +name = "mssql-python" +version = "1.15.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "azure-identity" }, + { name = "mssql-python-odbc" }, +] +wheels = [ + { url = "https://files.pythonhosted.org/packages/9d/0e/1c879dcab6f73e521b734f9224ac68b394958f267e6a278b76f6fae640d0/mssql_python-1.15.0-cp313-cp313-macosx_15_0_universal2.whl", hash = "sha256:2e9d78861d4f97d779ad770b9b4af26c1aa332e2d877b9df3c8896fe766968c6", size = 9960882, upload-time = "2026-09-11T15:22:13.403Z" }, + { url = "https://files.pythonhosted.org/packages/85/26/40b7cd3a6cb5ae13b3278192f31df3fa2ef3e153255d56662d71da2ae772/mssql_python-1.15.0-cp313-cp313-manylinux_2_28_aarch64.whl", hash = "sha256:1614c5d32fe5ddf30bb5a576412280eeb588a7bf371a6a81de3588c741059c13", size = 7130021, upload-time = "2026-09-11T15:22:15.609Z" }, + { url = "https://files.pythonhosted.org/packages/75/c7/591359e2ce07bf8024d372484be8fbb58d292425381763487d16f2fab626/mssql_python-1.15.0-cp313-cp313-manylinux_2_28_x86_64.whl", hash = "sha256:384d5729eed457047cfb3b506f489950700f10d285370d388312807b3bcffa17", size = 7913266, upload-time = "2026-09-11T15:22:17.198Z" }, + { url = "https://files.pythonhosted.org/packages/fd/aa/275d459975bc06b92826226e1f631e8a6e59e27325927bc487a8d16abf49/mssql_python-1.15.0-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:f92146ce5ba3f52838041df2cc699be508a2632fc50c25599844f7b50a016fb4", size = 6872555, upload-time = "2026-09-11T15:22:19.274Z" }, + { url = "https://files.pythonhosted.org/packages/59/e6/8f9825141b9875728cba0a5f402ad408e8fbab741412c6747e98b021e639/mssql_python-1.15.0-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:e16bcad380466cc6232852d372f48ef4c23c981f4d5e12698957b398c4bd04e2", size = 7585129, upload-time = "2026-09-11T15:22:21.266Z" }, + { url = "https://files.pythonhosted.org/packages/5e/2f/79ee4ec452e02af4fa3146818b976c1b00f0c8d8362faf031d292c4bca9a/mssql_python-1.15.0-cp313-cp313-win_amd64.whl", hash = "sha256:1f88f4497d2e40c75bfeb37b6fe0a790b661ac9049dd85f4a5b85143c2cd1f68", size = 5183190, upload-time = "2026-09-11T15:22:23.028Z" }, + { url = "https://files.pythonhosted.org/packages/e1/8b/a51b4485b303aa536a9fec1a84ac5927b81932a9764e4cabb0645f7933ab/mssql_python-1.15.0-cp313-cp313-win_arm64.whl", hash = "sha256:7b08b2906ff7339f9fb1752b21a6f25a8a00c36fc311a1371b9ca4c2bd66a0e3", size = 4930532, upload-time = "2026-09-11T15:22:24.855Z" }, + { url = "https://files.pythonhosted.org/packages/cc/fc/0942797afac9b2cf74b59ce3255364040ba3f50ba551c701df6e6262ffdb/mssql_python-1.15.0-cp314-cp314-macosx_15_0_universal2.whl", hash = "sha256:6b4f7826e22e7c0ae8613cf78509a81ce4f5d95f7a16b10493bc073653bb0176", size = 9971965, upload-time = "2026-09-11T15:22:27.039Z" }, + { url = "https://files.pythonhosted.org/packages/ce/6a/361e2e7fd7e1fccfcc3e7e56ab6793b0018b4b61597e6afd21171a350c47/mssql_python-1.15.0-cp314-cp314-manylinux_2_28_aarch64.whl", hash = "sha256:061670881c8b6f395aa1dad606fe2ff68af579367f91effde436e9f10042152d", size = 7679594, upload-time = "2026-09-11T15:22:29.259Z" }, + { url = "https://files.pythonhosted.org/packages/c7/bf/50d2e0911f383038e59cda0730c52a82a1fcd0763d084ebefd6b3f5387cd/mssql_python-1.15.0-cp314-cp314-manylinux_2_28_x86_64.whl", hash = "sha256:69640f8ea7726e9c1d7b673098baec4b7b637bb5b144764e2e688cade664064d", size = 8642553, upload-time = "2026-09-11T15:22:31.258Z" }, + { url = "https://files.pythonhosted.org/packages/0c/67/5fb9ec913454cddd4a27ce9e142b1b5edc01d96dec25e8ec2c15ef12311f/mssql_python-1.15.0-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:dcb141191daf6214f614907e40de2559aba9ff97f570ab2d4e3c1f02958c7e76", size = 7359238, upload-time = "2026-09-11T15:22:35.151Z" }, + { url = "https://files.pythonhosted.org/packages/96/c9/f3bdb4bf3fcac6102abdc4194a4b43b77a8da155e69c4c8aa1c18dbce265/mssql_python-1.15.0-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:c7305c55ba30bb9135e7273ffcb6624a1d25546c8b09033e12d9631d5f1e8e65", size = 8240128, upload-time = "2026-09-11T15:22:36.92Z" }, + { url = "https://files.pythonhosted.org/packages/af/e8/0793bdcd7016adf52c8f39ac8e1abd1f0eeef658c75d25e90ecc36a3a733/mssql_python-1.15.0-cp314-cp314-win_amd64.whl", hash = "sha256:cf265057da5337aa134358b97f80b0e4a118661848ccaff52db61d912c36d6f5", size = 5407508, upload-time = "2026-09-11T15:22:38.51Z" }, + { url = "https://files.pythonhosted.org/packages/6f/f3/175854dc9ca8ad74cd1823c94d82cfbd58b8f9602401f056f834245125d6/mssql_python-1.15.0-cp314-cp314-win_arm64.whl", hash = "sha256:f91c5d9027ed044d0d954525a05ce87fbcc5ac5a9c02072f0e50e391f37ed3dc", size = 5167181, upload-time = "2026-09-11T15:43:51.102Z" }, +] + +[[package]] +name = "mssql-python-odbc" +version = "18.6.2.1" +source = { registry = "https://pypi.org/simple" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e2/21/5d8e34820d986426dc65f88702ccecd1a981184e6bffab2968011461eaef/mssql_python_odbc-18.6.2.1-py3-none-macosx_15_0_universal2.whl", hash = "sha256:c0c4d2446f37656be1ed32c8e3229233554c1fbfc9f23c73c2ab0d07a2bfdcf2", size = 2035381, upload-time = "2026-08-05T15:51:46.762Z" }, + { url = "https://files.pythonhosted.org/packages/85/98/e07617483dd395156fba003cda6ff85ca9ad64de1347b823951f2afb3d60/mssql_python_odbc-18.6.2.1-py3-none-manylinux_2_28_aarch64.whl", hash = "sha256:d665529c195829970dcb72b7b3200e3c0ee1b053bee88e9a6eabaa385874dd6e", size = 2730032, upload-time = "2026-08-05T15:51:48.6Z" }, + { url = "https://files.pythonhosted.org/packages/35/1f/6cdb575549fbbe6f49bcadb505768649c43226bd83ec57a53a934d48e459/mssql_python_odbc-18.6.2.1-py3-none-manylinux_2_28_x86_64.whl", hash = "sha256:b8aad821f3b8637b0e6d4637f4b730ed9897c4665fa99d47e44e97fd1c79c77a", size = 3897879, upload-time = "2026-08-05T15:51:50.134Z" }, + { url = "https://files.pythonhosted.org/packages/89/ed/fa18a5f48eb4f0f003010af808b032806082c5441a076d698835ed5151b6/mssql_python_odbc-18.6.2.1-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:bcfca2a994b17499dc24a8b40228b851ccd0cd5807def855a6c41a8cedc4f8ac", size = 2730032, upload-time = "2026-08-05T15:51:51.672Z" }, + { url = "https://files.pythonhosted.org/packages/6c/66/7c96d719dfe7e9dcbbbf7acb345c72dfaef392fa6c5042e2718dfb1779b4/mssql_python_odbc-18.6.2.1-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:3db256103b09156f0980bee2dedc47995f528f76d01388ab5c5f1758b7e8fc53", size = 3897877, upload-time = "2026-08-05T15:51:53.147Z" }, + { url = "https://files.pythonhosted.org/packages/bb/f6/e7725d5dc791f5535fa7ec750c2ab9a1ff731f923d01b9c0d5cf1779b676/mssql_python_odbc-18.6.2.1-py3-none-win_amd64.whl", hash = "sha256:07a987221f99b368161db912065ef9e64c85f6b0901a2b05e9c1e6b41da08d55", size = 3712266, upload-time = "2026-08-05T15:51:54.589Z" }, + { url = "https://files.pythonhosted.org/packages/ea/d8/cda7ae46dd3c723196043109204fcc7749d688194de834b3f8aa4cd94415/mssql_python_odbc-18.6.2.1-py3-none-win_arm64.whl", hash = "sha256:00e1eae3c44a157ca60f992c6305be45856f18276ccff5258c1e9619847f3a5a", size = 7017266, upload-time = "2026-08-05T15:51:56.958Z" }, +] + [[package]] name = "multidict" version = "6.7.0" @@ -2282,6 +2321,7 @@ version = "0.1.0" source = { virtual = "." } dependencies = [ { name = "core" }, + { name = "mssql-python" }, { name = "opentelemetry-distro" }, { name = "opentelemetry-exporter-otlp-proto-http" }, { name = "pandas" }, @@ -2308,12 +2348,13 @@ dev = [ requires-dist = [ { name = "core", directory = "../PIXL/pixl_core" }, { name = "coverage", marker = "extra == 'coverage'", specifier = ">=7.0" }, + { name = "mssql-python", specifier = ">=1.13.0" }, { name = "opentelemetry-distro", specifier = "==0.63b1" }, { name = "opentelemetry-exporter-otlp-proto-http", specifier = "==1.42.1" }, { name = "pandas", specifier = "==2.3.2" }, { name = "pika", specifier = ">=1.3.2" }, { name = "pre-commit", specifier = ">=4.5.0" }, - { name = "psycopg2-binary", specifier = ">=2.9.10" }, + { name = "psycopg2-binary", specifier = ">=2.9.11" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.2" }, { name = "pytest-cov", marker = "extra == 'dev'", specifier = ">=7.1.0" }, { name = "requests", specifier = "==2.33.0" }, From 3d22cba88188d0ebf3995c9c7e0a9d456c294245 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 19:22:12 +0100 Subject: [PATCH 55/62] Delete pointless test that doesn't test anything --- tests/test_db_mssql.py | 63 ------------------------------------------ 1 file changed, 63 deletions(-) delete mode 100644 tests/test_db_mssql.py diff --git a/tests/test_db_mssql.py b/tests/test_db_mssql.py deleted file mode 100644 index 3928b3c..0000000 --- a/tests/test_db_mssql.py +++ /dev/null @@ -1,63 +0,0 @@ -from unittest.mock import MagicMock - -import pytest - -import db_mssql - - -def test_connect_uses_mssql_connection_string_and_timeouts(monkeypatch): - monkeypatch.setattr(db_mssql.settings, "CABOODLE_TESTING", "FALSE") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_HOST", "sql.example.com") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_PORT", "1433") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_DBNAME", "Caboodle") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_USERNAME", "reader") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_PASSWORD", "p;ass}") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_CONNECT_TIMEOUT", "10") - monkeypatch.setattr(db_mssql.settings, "CABOODLE_QUERY_TIMEOUT", "3001") - connection = MagicMock() - connect = MagicMock(return_value=connection) - monkeypatch.setattr(db_mssql.mssql_python, "connect", connect) - - database = db_mssql.caboodleDB() - database.connect() - - connect.assert_called_once_with( - ( - "Server={sql.example.com,1433};Database={Caboodle};" - "UID={reader};PWD={p;ass}}};" - ), - timeout=4, - attrs_before={db_mssql.mssql_python.SQL_ATTR_LOGIN_TIMEOUT: 10}, - ) - assert database.db_connection is connection - - -def test_get_rows_executes_named_parameters(): - cursor = MagicMock() - cursor.fetchall.return_value = [("row",)] - connection = MagicMock() - connection.cursor.return_value.__enter__.return_value = cursor - database = db_mssql.caboodleDB() - database.db_connection = connection - parameters = {"csn": "123"} - - rows = database._get_rows("SELECT * FROM Encounter WHERE CSN = %(csn)s", parameters) - - cursor.execute.assert_called_once_with( - "SELECT * FROM Encounter WHERE CSN = %(csn)s", parameters - ) - assert rows == [("row",)] - - -def test_get_rows_translates_operational_errors(): - cursor = MagicMock() - cursor.execute.side_effect = db_mssql.mssql_python.OperationalError( - "timed out", "timed out" - ) - connection = MagicMock() - connection.cursor.return_value.__enter__.return_value = cursor - database = db_mssql.caboodleDB() - database.db_connection = connection - - with pytest.raises(ConnectionError, match="Database error"): - database._get_rows("SELECT 1", {}) From 39d5be6f2d3aa64ed209aa2c3ec5b9d20cd714b8 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Mon, 14 Sep 2026 19:51:16 +0100 Subject: [PATCH 56/62] Define all Caboodle timeouts in seconds to match API --- config.EXAMPLE/exporter.env.EXAMPLE | 2 +- src/db_mssql.py | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/config.EXAMPLE/exporter.env.EXAMPLE b/config.EXAMPLE/exporter.env.EXAMPLE index 89b736c..8a1404d 100644 --- a/config.EXAMPLE/exporter.env.EXAMPLE +++ b/config.EXAMPLE/exporter.env.EXAMPLE @@ -40,7 +40,7 @@ CABOODLE_PASSWORD="inform" CABOODLE_HOST="localhost" CABOODLE_PORT="1433" CABOODLE_CONNECT_TIMEOUT="10" # in seconds -CABOODLE_QUERY_TIMEOUT="3000" # in milliseconds +CABOODLE_QUERY_TIMEOUT="10" # in seconds # To avoid having to deploy a fake caboodle for testing we have # a testing flag for Caboodle. If set TRUE caboodle connection will diff --git a/src/db_mssql.py b/src/db_mssql.py index e9299f2..14dafc0 100644 --- a/src/db_mssql.py +++ b/src/db_mssql.py @@ -1,5 +1,4 @@ from datetime import datetime -from math import ceil import mssql_python import pandas as pd @@ -37,14 +36,9 @@ def connect(self) -> None: self.fake_caboodle = settings.CABOODLE_TESTING == "TRUE" if not self.fake_caboodle: self.connection_string = _get_connection_string() - # CABOODLE_QUERY_TIMEOUT is configured in milliseconds, while - # mssql-python accepts whole seconds. - query_timeout_seconds = ceil( - int(settings.CABOODLE_QUERY_TIMEOUT) / 1000 # type:ignore - ) self.db_connection = mssql_python.connect( self.connection_string, - timeout=query_timeout_seconds, + timeout=settings.CABOODLE_QUERY_TIMEOUT, attrs_before={ mssql_python.SQL_ATTR_LOGIN_TIMEOUT: int( settings.CABOODLE_CONNECT_TIMEOUT # type:ignore From 523407f7be1ecd1d0f2330cd8da91964898529e0 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Tue, 15 Sep 2026 12:10:59 +0100 Subject: [PATCH 57/62] timeout needs to be an int --- src/db_mssql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db_mssql.py b/src/db_mssql.py index 14dafc0..2199b7d 100644 --- a/src/db_mssql.py +++ b/src/db_mssql.py @@ -38,7 +38,7 @@ def connect(self) -> None: self.connection_string = _get_connection_string() self.db_connection = mssql_python.connect( self.connection_string, - timeout=settings.CABOODLE_QUERY_TIMEOUT, + timeout=int(settings.CABOODLE_QUERY_TIMEOUT), attrs_before={ mssql_python.SQL_ATTR_LOGIN_TIMEOUT: int( settings.CABOODLE_CONNECT_TIMEOUT # type:ignore From 43127891df065623a3fb68eb94c5a5f3dc9e48e5 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Tue, 15 Sep 2026 12:24:48 +0100 Subject: [PATCH 58/62] Was getting runtime type error in pd.concat. Now always convert query results to a DataFrame, then access it as needed --- src/db_pg.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/db_pg.py b/src/db_pg.py index 0dd85f1..37bd304 100644 --- a/src/db_pg.py +++ b/src/db_pg.py @@ -46,7 +46,7 @@ def connect(self) -> None: def get_matched_mrn( self, location_string: str, observation_datetime: datetime - ) -> pd.DataFrame: + ) -> tuple: parameters = { "location_string": location_string, "observation_datetime": observation_datetime, @@ -55,14 +55,15 @@ def get_matched_mrn( "mrn_based_on_bed_and_datetime.sql", settings.SCHEMA_NAME ) - rows = self._get_rows(mrn_lookup_query, parameters) # type: ignore + rows = self._get_rows(mrn_lookup_query, parameters) - if len(rows) != 1: + num_rows = rows.shape[0] + if num_rows != 1: raise ValueError( - f"Wrong number of rows returned from database. {len(rows)} != 1, for {location_string}:{observation_datetime}" + f"Wrong number of rows returned from database. {num_rows} != 1, for {location_string}:{observation_datetime}" ) - return rows[0] + return tuple(rows.iloc[0]) def get_hospital_visit_from_csn(self, csn: str) -> int: hv_query = get_sql_query_with_schema( @@ -76,14 +77,7 @@ def get_hospital_visit_from_csn(self, csn: str) -> int: return 12345678 hospital_visit_id = self._get_rows(hv_query, parameters) - - # fetchall returns a list of tuples. We want the first element of the first tuple - if not isinstance(hospital_visit_id[0][0], int): - logger.warning( - f"hospital_visit_id[0][0] is not integer {hospital_visit_id}" - ) - - return hospital_visit_id[0][0] + return int(hospital_visit_id["hospital_visit_id"].iloc[0]) def get_flowsheets( self, start_datetime: datetime, end_datetime: datetime, hospital_visit_id: int @@ -141,14 +135,15 @@ def get_lab_results( return self._get_rows(lab_result_query, parameters) - def _get_rows(self, sql_query: sql.Composable, parameters: dict): + def _get_rows(self, sql_query: sql.Composable, parameters: dict) -> pd.DataFrame: try: with self.connection_pool.getconn() as db_connection: with db_connection.cursor() as curs: curs.execute(sql_query, parameters) rows = curs.fetchall() + col_names = [col.name for col in curs.description] self.connection_pool.putconn(db_connection) except psycopg2.errors.OperationalError as e: self.connection_pool.putconn(db_connection) raise ConnectionError(f"Data base error: {e}") - return rows + return pd.DataFrame(rows, columns=col_names) From d2a67755603c3f905e068794b89745586f08e2c4 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Tue, 15 Sep 2026 12:30:02 +0100 Subject: [PATCH 59/62] Fix typo in SQL query --- src/sql/flow_sheet_values.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sql/flow_sheet_values.sql b/src/sql/flow_sheet_values.sql index 7076491..52be34b 100644 --- a/src/sql/flow_sheet_values.sql +++ b/src/sql/flow_sheet_values.sql @@ -40,7 +40,7 @@ LEFT JOIN {schema_name}.visit_observation_type AS vt WHERE vt.id_in_application IN ('6', '3040102622', '12946', '40191', '39947') - AND vo.valid_from >= %(start_datetime)s AND vo.valid_from < %(end$_datetime)s + AND vo.valid_from >= %(start_datetime)s AND vo.valid_from < %(end_datetime)s AND vo.hospital_visit_id = %(hospital_visit_id)s GROUP BY "DateTimeRecorded", "Units" From 3a85bc924b3454b919b4bbb14f58e335a8621d49 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Tue, 15 Sep 2026 12:40:53 +0100 Subject: [PATCH 60/62] Also return DataFrame for MSSQL queries --- src/db_mssql.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/db_mssql.py b/src/db_mssql.py index 2199b7d..c66cf17 100644 --- a/src/db_mssql.py +++ b/src/db_mssql.py @@ -69,12 +69,13 @@ def get_airflow( return self._get_rows(airway_query, parameters) - def _get_rows(self, sql_query: str, parameters: dict): + def _get_rows(self, sql_query: str, parameters: dict) -> pd.DataFrame: try: with self.db_connection.cursor() as curs: curs.execute(sql_query, parameters) rows = curs.fetchall() + col_names = [col.name for col in curs.description] except mssql_python.OperationalError as e: raise ConnectionError(f"Database error: {e}") from e - return rows + return pd.DataFrame(rows, columns=col_names) From 1d5f07d79356ef493f7a50d0089ef83bc6c277da Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Tue, 15 Sep 2026 13:44:49 +0100 Subject: [PATCH 61/62] MSSQL column info is a bit different --- src/db_mssql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db_mssql.py b/src/db_mssql.py index c66cf17..84132a0 100644 --- a/src/db_mssql.py +++ b/src/db_mssql.py @@ -74,7 +74,7 @@ def _get_rows(self, sql_query: str, parameters: dict) -> pd.DataFrame: with self.db_connection.cursor() as curs: curs.execute(sql_query, parameters) rows = curs.fetchall() - col_names = [col.name for col in curs.description] + col_names = [col[0] for col in curs.description] except mssql_python.OperationalError as e: raise ConnectionError(f"Database error: {e}") from e From 76989a9e93eab95cf942e94ee298b789732d0b31 Mon Sep 17 00:00:00 2001 From: Jeremy Stein Date: Tue, 15 Sep 2026 13:58:35 +0100 Subject: [PATCH 62/62] Update the list of safe EHR columns to match the queries --- src/electronic_health_records/ehr.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/electronic_health_records/ehr.py b/src/electronic_health_records/ehr.py index 5a44eac..fe2922b 100644 --- a/src/electronic_health_records/ehr.py +++ b/src/electronic_health_records/ehr.py @@ -71,16 +71,18 @@ def _ehr_for_csv( "PlacementInstant", "RemovalInstant", "TubeSize", + "Repositioned", + "Position frequency", "Temperature", "Noradrenaline", "Metaraminol", + "PaO2", + "PaCO2", + "Secretions", + "Sputum", "Units", - "Abnormal_result", - "C-reactive protein 1", - "CSF WCC TUBE 1", - "CSF WCC TUBE 2", - "CSF WCC TUBE 3", - "C-reactive protein 2", + "CRP", + "WCC", "Comments", # Free text comments could contain sensitive information. Should we hash it? ]