Skip to content

Commit

Permalink
fix: Temporarily revert to denylist for T1OO
Browse files Browse the repository at this point in the history
This reflects the new interim agreement with respect to T1OOs. This
change is only required while TPP update the logic for populating the
allowlist. Once that is complete we can switch back to using the
allowlist. For this reason I am leaving some of the end-to-end test
setup in place even though it is not strictly required while using the
denylist.

Reverts some of #2072
  • Loading branch information
evansd committed Oct 25, 2024
1 parent 930bdd7 commit 44585dc
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
28 changes: 14 additions & 14 deletions ehrql/backends/tpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,26 +101,28 @@ def modify_query_variables(self, variables):
return variables

# Otherwise we add an extra condition to the population definition which is that
# the patient appears in the table of "allowed" patients.
# the patient does not appear in the T1OO table.
#
# PLEASE NOTE: This logic is referenced in our public documentation, so if we
# make any changes here we should ensure that the documentation is kept
# up-to-date:
# https://github.com/opensafely/documentation/blob/ea2e1645/docs/type-one-opt-outs.md
#
# From ehrQL's point of view, the construction of the "allowed patients" table
# is opaque. For discussion of the approach currently used to populate this see:
# From ehrQL's point of view, the construction of the T1OO table is opaque. For
# discussion of the approach currently used to populate this see:
# https://docs.google.com/document/d/1nBAwDucDCeoNeC5IF58lHk6LT-RJg6YZRp5RRkI7HI8/
variables = dict(variables)
variables["population"] = qm.Function.And(
variables["population"],
qm.AggregateByPatient.Exists(
# We don't currently expose this table in the user-facing schema. If
# we did then we could avoid defining it inline like this.
qm.SelectPatientTable(
"allowed_patients",
# It doesn't need any columns: it's just a list of patient IDs
schema=qm.TableSchema(),
qm.Function.Not(
qm.AggregateByPatient.Exists(
# We don't currently expose this table in the user-facing schema. If
# we did then we could avoid defining it inline like this.
qm.SelectPatientTable(
"t1oo",
# It doesn't need any columns: it's just a list of patient IDs
schema=qm.TableSchema(),
)
)
),
)
Expand Down Expand Up @@ -154,10 +156,8 @@ def get_exit_status_for_exception(self, exception):
exception.add_note(f"\nDatabase error: {exception}")
return 5

allowed_patients = MappedTable(
# This table has its name for historical reasons, and reads slightly oddly: it
# should be interpreted as "allowed patients with regard to type one dissents"
source="AllowedPatientsWithTypeOneDissent",
t1oo = MappedTable(
source="PatientsWithTypeOneDissent",
# The allowed patients table doesn't need any columns: it's just a list of
# patient IDs
columns={},
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/backends/test_tpp.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
OPA,
OPA_ARCHIVED,
UKRR,
AllowedPatientsWithTypeOneDissent,
APCS_Cost,
APCS_Cost_ARCHIVED,
APCS_Cost_JRC20231009_LastFilesToContainAllHistoricalCostData,
Expand Down Expand Up @@ -52,6 +51,7 @@
Organisation,
Patient,
PatientAddress,
PatientsWithTypeOneDissent,
PotentialCareHomeAddress,
RegistrationHistory,
Relationship,
Expand Down Expand Up @@ -3112,8 +3112,8 @@ def test_t1oo_patients_excluded_as_specified(mssql_database, suffix, expected):
Patient(Patient_ID=2, DateOfBirth=date(2002, 1, 1)),
Patient(Patient_ID=3, DateOfBirth=date(2003, 1, 1)),
Patient(Patient_ID=4, DateOfBirth=date(2004, 1, 1)),
AllowedPatientsWithTypeOneDissent(Patient_ID=1),
AllowedPatientsWithTypeOneDissent(Patient_ID=4),
PatientsWithTypeOneDissent(Patient_ID=2),
PatientsWithTypeOneDissent(Patient_ID=3),
)

dataset = create_dataset()
Expand Down

0 comments on commit 44585dc

Please sign in to comment.