Expose consultation_id on TPP medications table
#1993
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
evansd
force-pushed
the
evansd/consultation-id
branch
from
7ee5414 to
2a2da76
Compare
Deploying databuilder-docs with
|
| Latest commit: |
9b6801d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b3299b55.databuilder.pages.dev |
| Branch Preview URL: | https://evansd-consultation-id.databuilder.pages.dev |
rebkwok
approved these changes
Apr 25, 2024
There was a problem hiding this comment.
Nice. I had an attempt at doing the table inheritance a while ago but couldn't figure out how to get it to work. There are probably some other tables that should be updated with it too (I think it was the clinical events table that i wanted to do it for before.
ehrql/tables/tpp.py
Outdated
| @table | ||
| class medications(ehrql.tables.core.medications.__class__): | ||
| """ | ||
| The medication table provides data about prescribed medications in primary care. |
There was a problem hiding this comment.
Suggested change
| The medication table provides data about prescribed medications in primary care. | |
| The medications table provides data about prescribed medications in primary care. |
This allows us to say: "this table here is just like this other one there but adds some extra columns." Auto-complete continues to work correctly and picks up the inherited columns.
Docstrings are a bit of a wrinkle here: they obviously aren't automatically inherited from the parent; we could set them dynamically by fiddling about with the `__doc__` attribute but that would prevent them showing up in VSCode, and it's quite a nice feature that they display directly in the editor. So instead we require them to be manually copy-pasted but we add a check that they are consistent to avoid drift. We don't require that they are identical because it seems reasonable to allow the child table to extend the parent docstring, but they must share a common prefix.
evansd
force-pushed
the
evansd/consultation-id
branch
from
2a2da76 to
9b6801d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This requires us to create a TPP specialisation of existing core table. Obviously we can just do this by copy/pasting but I worry that this risks drift in the documentation. (Drift in the table structure itself is less of a worry as the backends will naturally enforce consistency for tables sharing the same name.) So I've made a slight tweak to the
@tabledecorator to support table inheritance and added a mechanism to enforce docstring consistency.Closes #1983