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

Add ehrQL placeholders #106

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Conversation

milanwiedemann
Copy link
Member

@milanwiedemann milanwiedemann commented Sep 14, 2023

This adds a placeholer ehrql dataset definition and action with instructions for cohortextractor and ehrql users at the top of each script (dataset_definition.py and study_definition.py).

This will be helpful for creating ehrQL documentation and examples because if gives us a starting point that is easy for users to get to. It's also a good to introduce ehrql in the research template simply because we are moving away from cohortextractor.

The dataset_definition.py is not a direct translation of the study_definition.py because I think it's good to use an ehrql method (age_on) in the example and show how a population can be defined using two conditions.

However, we should have think if this is a good starting point for all studies that use the research-template or whether we want to make it more simple/complex?

This is not a direct translation of the cohort-extractor example in study_definition.py.

I thought it would be useful to show how to add a column to the dataset using an ehrql method (age_on) and show how to use two conditions to define the pupulation using &.
This adds instruction to the study_population.py and dataset_definition.py files for cohort-extractor vs ehrql users.
@inglesp
Copy link
Contributor

inglesp commented Sep 15, 2023

However, we should have think if this is a good starting point for all studies that use the research-template or whether we want to make it more simple/complex?

Maybe? But we don't need to decide that now.

Do we need to update anything in the docs?

@milanwiedemann
Copy link
Member Author

Maybe? But we don't need to decide that now.

Agree!

Do we need to update anything in the docs?

At this stage I don't think we have to change anything. I looked at the docs and the research template gets a lot of mentions but because we didn't change the default behaviour I think we can merge this without making changes to the docs!

@milanwiedemann milanwiedemann marked this pull request as ready for review September 15, 2023 14:56
@milanwiedemann milanwiedemann merged commit fe9f7ef into main Oct 13, 2023
1 check passed
@milanwiedemann milanwiedemann deleted the milanwiedemann/add-ehrql-placeholders branch October 13, 2023 09:52
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.

Update project template to include ehrQL placeholders
2 participants