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

Viv3ckj/move validation data #90

Merged
merged 7 commits into from
Jan 8, 2025
Merged

Viv3ckj/move validation data #90

merged 7 commits into from
Jan 8, 2025

Conversation

viv3ckj
Copy link
Contributor

@viv3ckj viv3ckj commented Jan 6, 2025

Closes #84

@viv3ckj viv3ckj requested a review from milanwiedemann January 6, 2025 16:10
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.

Great work! I'm not sure about the denominator for the new measures pfmed_on_pfdate, pfpathway_on_pfdate, pfmed_and_pfpathway_on_pfdate. Was there a reason we discussed why we wanted to exclude pf_consultation?

analysis/measures_definition_pf_descriptive_stats.py Outdated Show resolved Hide resolved
analysis/measures_definition_pf_descriptive_stats.py Outdated Show resolved Hide resolved
pharmacy_first_dates = select_events(clinical_events, codelist=pharmacy_first_consultation_codelist).date

# Specify whether a patient has been prescribed a PF medication on the same day as a PF consultation code
has_pfmed_on_pfdate = selected_med_events.where(medications.date.is_in(pharmacy_first_dates)).exists_for_patient()
Copy link
Member

@milanwiedemann milanwiedemann Jan 7, 2025

Choose a reason for hiding this comment

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

selected_med_events has already been filtered to only include those events where the consultation_id matches a pharmacy_first_ids, see:

selected_med_events = select_events(
    medications, consultation_ids=pharmacy_first_ids).where(
    medications.date.is_on_or_between(INTERVAL.start_date, INTERVAL.end_date)
)

here we want to use the date as a backup to look at all pharmacy first medications on the same day as a pharmacy first consultation, specifically for those events where the linkage through consultation_id doesnt exist. so we need to provide a medications event frame that still includes all the pf medications

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot! Thanks, I've made the changes now.

@milanwiedemann
Copy link
Member

We just noticed that we need to rework some of the selected events because we sometimes want them linked by consultation_id and sometimes not so that we can link them by date of the pharmacy first consultation, see https://github.com/opensafely/pharmacy-first/pull/90/files#r1905757995

@milanwiedemann
Copy link
Member

milanwiedemann commented Jan 8, 2025

@viv3ckj I just wanted to check my understanding why we're not pulling the eps_erd_prescribing_data.r function from the data development project. I think that was because we're not expecting to continue looking at medication status in this repo?

@viv3ckj
Copy link
Contributor Author

viv3ckj commented Jan 8, 2025

@milanwiedemann we are pulling eps_erd_prescribing_data.R, check lib/functions/eps_erd_prescribing_data.R.

Do you think it should or shouldn't be included?

@viv3ckj viv3ckj merged commit 2ac4e21 into main Jan 8, 2025
1 check passed
@viv3ckj viv3ckj deleted the viv3ckj/move_validation_data branch January 8, 2025 13:47
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.

Move code from data development project
2 participants