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.

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

Viv3ckj/statistics #66

Merged
merged 5 commits into from
Nov 21, 2024
Merged

Viv3ckj/statistics #66

merged 5 commits into from
Nov 21, 2024

Conversation

viv3ckj
Copy link
Contributor

@viv3ckj viv3ckj commented Nov 21, 2024

Closes #50

@viv3ckj viv3ckj mentioned this pull request Nov 21, 2024
Closed
3 tasks
milanwiedemann
milanwiedemann reviewed Nov 21, 2024
View reviewed changes
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.

We should restructure the script and make it easier to read by following a consistent structure. We should overwrite denominator as it now means 2 different things depending on where we are in script. Also I think we should have all the code first, then define all measures together at the end of the script?

milanwiedemann
milanwiedemann approved these changes Nov 21, 2024
View reviewed changes
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.

Thanks, this looks great! all that needed now is to remove all changes from analysis/measures_definition_pf_breakdown.py. Have a look at the suggestions I made in the new measures definition. Mostly small nitpicks, see what you think!

analysis/measures_definition_pf_breakdown.py Outdated Show resolved Hide resolved
analysis/measures_definition_pf_statistics.py Outdated Show resolved Hide resolved
analysis/measures_definition_pf_statistics.py Outdated Show resolved Hide resolved
analysis/measures_definition_pf_statistics.py Outdated Show resolved Hide resolved
analysis/measures_definition_pf_statistics.py Outdated Show resolved Hide resolved
analysis/measures_definition_pf_statistics.py Outdated Show resolved Hide resolved
project.yaml Outdated Show resolved Hide resolved
project.yaml Outdated Show resolved Hide resolved
@viv3ckj viv3ckj merged commit 00ce545 into main Nov 21, 2024
1 check passed
@viv3ckj viv3ckj deleted the viv3ckj/statistics branch November 21, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@milanwiedemann milanwiedemann milanwiedemann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add statistics describing medications and conditions of PF consultations
2 participants
@viv3ckj @milanwiedemann