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.

Sign up for GitHub

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

Jump to bottom

Add AllergyIntolerance support to core study #287

Merged

mikix merged 7 commits into main from mikix/allergy

Aug 27, 2024

Contributor

mikix commented Aug 26, 2024 •

edited

Loading

Adds core__allergyintolerance
Adds core__count_allergyintolerance_month

Fixes #285
A subset of #139

Checklist

Consider if documentation (like in docs/, or if you've changed the structure of a table) needs to be updated
Consider if tests should be added
Update template repo if there are changes to study configuration

mikix commented

View reviewed changes

cumulus_library/statistics/statistics_templates/count.sql.jinja Show resolved Hide resolved

mikix commented

View reviewed changes

cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja Show resolved Hide resolved

mikix commented

View reviewed changes

cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja Show resolved Hide resolved

mikix force-pushed the mikix/allergy branch 3 times, most recently from 28cd261 to e7f0779 Compare

August 26, 2024 17:05

mikix commented

View reviewed changes

cumulus_library/studies/core/core_templates/allergyintolerance.sql.jinja Show resolved Hide resolved

mikix commented

View reviewed changes

cumulus_library/studies/core/core_templates/allergyintolerance.sql.jinja Show resolved Hide resolved

mikix commented

View reviewed changes

cumulus_library/studies/core/core_templates/allergyintolerance.sql.jinja Show resolved Hide resolved

mikix force-pushed the mikix/allergy branch from e7f0779 to 3533ce5 Compare

August 26, 2024 17:26

github-actions bot commented Aug 26, 2024 •

edited

Loading

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
2020	2006	99%	90%	🟢

New Files

File	Coverage	Status
cumulus_library/studies/core/builder_allergyintolerance.py	100%	🟢
TOTAL	100%	🟢

Modified Files

File	Coverage	Status
cumulus_library/init.py	100%	🟢
cumulus_library/statistics/counts.py	100%	🟢
cumulus_library/statistics/statistics_templates/counts_templates.py	100%	🟢
cumulus_library/studies/core/count_core.py	100%	🟢
TOTAL	100%	🟢

updated for commit: 796ca83 by action🐍


          Add AllergyIntolerance support to core study

69a194c

- Adds core__allergyintolerance
- Adds core__count_allergyintolerance_month

mikix force-pushed the mikix/allergy branch from 3533ce5 to 69a194c Compare

August 26, 2024 17:48

mikix commented

View reviewed changes

tests/test_counts_templates.py Show resolved Hide resolved

mikix marked this pull request as ready for review

August 26, 2024 18:00

dogversioning approved these changes

View reviewed changes

cumulus_library/studies/core/core_templates/allergyintolerance.sql.jinja Show resolved Hide resolved

cumulus_library/studies/core/core_templates/allergyintolerance.sql.jinja Show resolved Hide resolved

cumulus_library/studies/core/builder_allergyintolerance.py Show resolved Hide resolved

cumulus_library/studies/core/reference_sql/builder_observation.sql Show resolved Hide resolved

cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja Show resolved Hide resolved

docs/core-study-details.md Outdated Show resolved Hide resolved

tests/core/test_core.py Outdated Show resolved Hide resolved

tests/test_counts_templates.py Show resolved Hide resolved

cumulus_library/studies/core/core_templates/allergyintolerance.sql.jinja Show resolved Hide resolved

mikix added 5 commits

August 27, 2024 09:03


          Ignore bsv files when detecting repo language

7ffe8ae


          Update optional fields list in docs

812a2a0


          Make test strings more verbose/clear

8cc39f9


          Switch allergyintolerance counts to by-patient (vs by-resource)

26c33db


          Require allergyintolerance in completion tracking

38a1dab

Contributor Author

mikix commented Aug 27, 2024

OK, I just added a few commits, separated out for easier reviewing:

Have GitHub ignore *.bsv files when detecting language
Update the docs (and the PR template) around non-US-Core ("optional") fields in the docs - both removed fields referenced by US Core and added missing fields we use that aren't in US Core
Addressed other review comments
Switched the default AllergyIntolerance count to be by-patient and added test around that
Updated the completion tracking to require AllergyIntolerance too


          Update regression test for by-patient allergies

796ca83

dogversioning reviewed

View reviewed changes

cumulus_library/statistics/counts.py

Comment on lines +9 to +11

		# Defined here for easy overriding by tests
		DEFAULT_MIN_SUBJECT = 10

Contributor

dogversioning Aug 27, 2024

ooh, this is a very good idea

Contributor

dogversioning commented Aug 27, 2024

OK, I just added a few commits, separated out for easier reviewing:

Took a scan, looks good

mikix merged commit c1d62ad into main

3 checks passed

mikix deleted the mikix/allergy branch

August 27, 2024 16:35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet