-
Notifications
You must be signed in to change notification settings - Fork 17
Add monthly daly reporting to individual history tracker #1847
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
55adae7
2bcab12
c712997
6ae6bfd
a3e43bf
cf79a8e
dffe729
9e094db
a16a4c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |||||
| get_gbd_causes_not_represented_in_disease_modules, | ||||||
| ) | ||||||
| from tlo.methods.demography import age_at_date | ||||||
| from tlo.notify import notifier | ||||||
|
|
||||||
| logger = logging.getLogger(__name__) | ||||||
| logger.setLevel(logging.INFO) | ||||||
|
|
@@ -614,6 +615,20 @@ def apply(self, population): | |||||
| # Multiply 1/12 as these weights are for one month only | ||||||
| disease_specific_daly_values_this_month = disease_specific_daly_values_this_month * (1 / 12) | ||||||
|
|
||||||
| if notifier.has_listeners("healthburden.monthly_daly_report"): | ||||||
| # Do not dispatch individuals or causes that have zero dalys reported this month | ||||||
| monthly_dalys = disease_specific_daly_values_this_month.copy() | ||||||
| monthly_dalys_nonzero = ( | ||||||
| monthly_dalys.loc[(monthly_dalys != 0).any(axis=1), (monthly_dalys != 0).any(axis=0)] | ||||||
| .to_dict(orient="index")) | ||||||
|
|
||||||
| # Store data as dictionary | ||||||
| data = { | ||||||
| person: {col: val for col, val in cols.items() if val != 0} | ||||||
| for person, cols in monthly_dalys_nonzero.items() | ||||||
| } | ||||||
| notifier.dispatch("healthburden.monthly_daly_report", data=data) | ||||||
|
Comment on lines
+625
to
+630
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering why you pack it into a dictionary here (two nested for-loops), and then unpack again (two more nested for-loops) when it goes to the function in the notifier? Could the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes that's a good point; my naive guess was that the cost of reformatting non-zero entries into a dictionary is less than dispatching the entire dataframe which may contain a lot of zero entries*, but this may be incorrect. @tamuri happy for you to advise. *This also a naive guess, based on the assumption that most individuals only have one or two conditions reported every months (meaning that all other columns would be zero for them), and that a single condition would typically affect <<20% (?) of the population.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tim is right - you don't need any filtering here. You're passing a reference, not a copy of the dataframe. You do need to ensure there is no editing of the dataframe in the receiver because it is used after the notification. It's also how notifications are usually setup. You don't know how receivers need the information so send everything and then the receiver can do its own filtering (or not, as needed).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An additional benefit is that you can remove the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use dash instead of underscores here, to match other listeners.
Suggested change
|
||||||
|
|
||||||
| # 4) Summarise the results for this month wrt sex/age/wealth | ||||||
| # - merge in age/wealth/sex information | ||||||
| disease_specific_daly_values_this_month = disease_specific_daly_values_this_month.merge( | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.