Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added breakdown graphs and facet function #11

Merged
merged 6 commits into from
Oct 1, 2024
Merged

Conversation

viv3ckj
Copy link
Contributor

@viv3ckj viv3ckj commented Sep 26, 2024

Closes #10
Closes #12
Closes #13
Closes #14

@viv3ckj viv3ckj marked this pull request as ready for review September 26, 2024 14:16
@viv3ckj viv3ckj changed the title added age groups and facet function added breakdown graphs and facet function Sep 30, 2024
Copy link
Member

@milanwiedemann milanwiedemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellet! Code looks like it's doing exactly what we want, but I think we can make some sections a bit shorter

Comment on lines +32 to +40
age = patients.age_on(INTERVAL.start_date)
age_band = case(
when((age >= 0) & (age < 20)).then("0-19"),
when((age >= 20) & (age < 40)).then("20-39"),
when((age >= 40) & (age < 60)).then("40-59"),
when((age >= 60) & (age < 80)).then("60-79"),
when(age >= 80).then("80+"),
when(age.is_null()).then("Missing"),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

when(imd < int(32844 * 3 / 5)).then("3"),
when(imd < int(32844 * 4 / 5)).then("4"),
when(imd < int(32844 * 5 / 5)).then("5 (least deprived)"),
otherwise="unknown"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although the code does exactly the same as this suggestions in the tutorial, I'd use something like this because it's a bit easier to review and harder to make a mistake:

imd_rounded = addresses.for_patient_on(
    INTERVAL.start_date
).imd_rounded
max_imd = 32844
dataset.imd_quintile = case(
    when(imd_rounded < int(max_imd * 1 / 5)).then(1),
    when(imd_rounded < int(max_imd * 2 / 5)).then(2),
    when(imd_rounded < int(max_imd * 3 / 5)).then(3),
    when(imd_rounded < int(max_imd * 4 / 5)).then(4),
    when(imd_rounded <= max_imd).then(5),
)

when(imd < int(32844 * 3 / 5)).then("3"),
when(imd < int(32844 * 4 / 5)).then("4"),
when(imd < int(32844 * 5 / 5)).then("5 (least deprived)"),
otherwise="unknown"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be consistent with the value for "Missing" data, I'd use the same as for age bands here as well

Suggested change
otherwise="unknown"
otherwise="Missing"

Comment on lines +76 to +118
# Measures for age breakdown of clinical services
measures.define_measure(
name=f"count_{pharmacy_first_event}_by_age",
numerator=numerator,
denominator=denominator,
group_by={
"age_band": age_band,
},
intervals=months(monthly_intervals).starting_on(start_date),
)

# Measures for sex breakdown of clinical services
measures.define_measure(
name=f"count_{pharmacy_first_event}_by_sex",
numerator=numerator,
denominator=denominator,
group_by={
"sex": patients.sex,
},
intervals=months(monthly_intervals).starting_on(start_date),
)

# Measures for IMD breakdown of clinical services
measures.define_measure(
name=f"count_{pharmacy_first_event}_by_imd",
numerator=numerator,
denominator=denominator,
group_by={
"imd": imd_quintile,
},
intervals=months(monthly_intervals).starting_on(start_date),
)

# Measures for region breakdown of clinical services
measures.define_measure(
name=f"count_{pharmacy_first_event}_by_region",
numerator=numerator,
denominator=denominator,
group_by={
"region": registration.practice_nuts1_region_name,
},
intervals=months(monthly_intervals).starting_on(start_date),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks correct, but I think we can make this much shorter if we group by multiple features, see https://docs.opensafely.org/ehrql/explanation/measures/#grouping-by-multiple-features

Comment on lines 144 to 186
# Measures for age breakdown of clinical conditions
measures.define_measure(
name=f"count_{condition_name}_by_age",
numerator=numerator,
denominator=denominator,
group_by={
"age_band": age_band,
},
intervals=months(monthly_intervals).starting_on(start_date),
)

# Measures for age breakdown of clinical conditions
measures.define_measure(
name=f"count_{condition_name}_by_sex",
numerator=numerator,
denominator=denominator,
group_by={
"sex": patients.sex,
},
intervals=months(monthly_intervals).starting_on(start_date),
)

# Measures for imd breakdown of clinical conditions
measures.define_measure(
name=f"count_{condition_name}_by_imd",
numerator=numerator,
denominator=denominator,
group_by={
"imd": imd_quintile,
},
intervals=months(monthly_intervals).starting_on(start_date),
)

# Measures for region breakdown of clinical conditions
measures.define_measure(
name=f"count_{condition_name}_by_region",
numerator=numerator,
denominator=denominator,
group_by={
"region": registration.practice_nuts1_region_name,
},
intervals=months(monthly_intervals).starting_on(start_date),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment above, I think we can make this consierably shorter

@viv3ckj viv3ckj merged commit 0753306 into main Oct 1, 2024
1 check passed
@viv3ckj viv3ckj deleted the viv3ckj/report-metrics branch October 1, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add breakdown by region Add breakdown by IMD Add breakdown by sex Add breakdown by age
2 participants