-
Notifications
You must be signed in to change notification settings - Fork 16
Adding herbal medication to lifestyle module #1785
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?
Conversation
|
Hi @upile-n --- that sounds a great idea. I think you've haven't committed/pushed your changes onto this branch though, as the only change we see for now is a comment in the cardio-metabolic-disorders module. Please ping up when all the changes are here and ready for review. |
Hi @tbhallett we are still working on the actual changes and will push those once complete. I will let you know once it's ready for review. |
…tyle' into upile/herbal-medication_in_lifestyle # Conflicts: # src/tlo/methods/enhanced_lifestyle.py
|
Hello @tbhallett and @andrew-phillips-1. This PR is now complete, and ready for your review. |
|
This seems fine but I was thinking you could have it even simpler than this and have herbal medication use as a fixed personal property, like sex or urban/rural, so you just assign each person in 2010 or at birth with a herbal medication status that applies for life. |
Thank you for your prompt feedback. I guess in order to do that I need to delete the function "update_herbal_medication_property_linear_model()" and the start and stop parameters related to it. I think this should simplify is at you have stated? |
|
HI Upile Yes, but let's see what Tim and Wati think also ? I may be missing some benefits of how it is now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with Andrew!
- This is nicely coded and looks like it will work fine. WELL DONE!
- I'm intersetd in the decison to make it a property that can change versus one that is fixed for an individual. Is there a good reason to go one way or the other? Let's have a think and discuss
- For the PR, we'd also need to :
- have a look at plots showing what this property does - you've made a good start here. (prevalence of it, by urban/rural over time)
- have a justifcation for any new parameters included in the write-up document.
|
|
||
|
|
||
| class HSI_CardioMetabolicDisorders_Refill_Medication(HSI_Event, IndividualScopeEventMixin): | ||
| #This is an HSI for medication refill |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove incidental change (to keep the PR tidy)
| "li_is_circ": Property(Types.BOOL, "Is the person circumcised if they are male (False for all females)"), | ||
| "li_is_circ": Property(Types.BOOL, "Is the person circumcised if they are male (False for all females)" | ||
| ), | ||
| 'li_herbal_medication': Property(Types.BOOL, 'whether someone uses herbal medication or not'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add the word 'currently' in here (if we're to keep the current structure, of whether use can start and stop)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have justifications for these parameters?
Would be great for that to be included in an update to the write-up document to explain the thinking behind any parameter values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice figure generated that you've posted in the PR conversation. Are there other plots too? Would be interested in a trend over time, and also a comparison to data, if possible (is it possible!??!?)
(Also lots of things commented-out here. Not clear what you want to keep and what not.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will clean the code that has been commented-out. We were waiting for the initial review, but it'll be cleaned.
Other plots should be possible.
|
I think one thing to consider here is whether we are thinking of this property as "use of the types of herbal medications that cause increased renal problems" or something more general (I would vote for the former I think). I was seeing that there are various herbal medications used and not all are thought to contribute to renal problems. Once that is defined then perhaps Upile you can do another search (and great that you have access to all articles now through UCL if you need them) and look for estimates of prevalence by urban/rural, but I think you may well find none. It seems to be clear that use is highly differential by urban/rural but perhaps you can find some sources that state that, even if not quantitative. I think we are probably going to have to say that we have gone with target of 80% in rural areas because it is clear that use is widespread although not universal. Whilst the 10% in urban areas reflect that use is much more limited. We have assumed that it tends to be a behaviour associated with a person over their hole like and effects are long term so we elected to make it a fixed property of an individual ? Let me know if you disagree though. |
Having the start and stop parameters were mostly to consider people who would stop/start taking herbal medication for any reason... But you're right that it would be best not to include that here. The property li_herbal_medication is currently general in the lifestyle module, so perhaps modelling whether someone starts or stops herbal medication could come inside a specific disease module (i.e., chronic_kidney_disease) that will use the property if necessary. |
|
Yes if we think this is mainly about renal disease then perhaps the property can be in the kidney transplant module. |
I would opt for the more general approach so that this property can be used in other modules, and not just chronic kidney disease, whether now or in future. I am, however, open to discuss that further, if having specific types of herbal medication could have an added benefit. I will be sure to do a deep dive search for the urban/rural estimates. |

Fixes #1784
Introduce herbal medication use as a lifestyle exposure within the Lifestyle module, represented as a categorical or binary variable (e.g. current use / no use), with stratification by rural vs urban residence.