The `Equipment` Class #1098

EvaJanouskova · 2023-09-06T15:50:03Z

main changes:

modified healthsystem module to allow tracking and logging of equipment used

minor changes:

added some TODOs (hoping at least some of them will be resolved before this PR closes)

TODO

before merge:

rm the dummy examples before merge
what equip availability default values for the merge?

move to next PR:

open issue to include essential equipment if any in TLOmodel/src/tlo/analysis/hsi_events.py script to be shown on https://www.tlomodel.org/hsi_events.html

tbhallett

Great start on this Eva. Only minor comments.

Do we have an idea about how to provide people with a catalogue of equipment they can choose from when making these declarations?

src/scripts/healthsystem/equipment/equipment_catalogue.py

src/tlo/analysis/utils.py

src/tlo/methods/breast_cancer.py

tbhallett · 2023-09-12T11:18:21Z

src/tlo/methods/breast_cancer.py

@@ -646,6 +647,7 @@ def __init__(self, module, person_id):
        self.TREATMENT_ID = "BreastCancer_Investigation"
        self.EXPECTED_APPT_FOOTPRINT = self.make_appt_footprint({"Over5OPD": 1, "Mammography": 1})
        self.ACCEPTED_FACILITY_LEVEL = '3'  # Mammography only available at level 3 and above.
+        # TODO: what this means, should be the mammography done within this event, or the biopsy, or both?


Not sure about this. Better ask Andrew.

@andrew-phillips-1, Would you know what is done within this HSI? Biopsy, mammography, or both?

The guideline you sent me, says in 10.3.3 'Breast Cancer':
"Diagnosis -> Definitive diagnosis is through histology. This is more superior than cytology which should be understood as a preliminary diagnostic test."

However, in the beginning of 10.1 'Cancer Diagnosis and Registration' Section they mention Mammography as a radiological test for breast cancer.

@EvaJanouskova I don't know for sure but I think we should consider this as just biopsy. As you saw in the guidelines, population screening for mammography is not done. So I think the mammography can be removed here.

@andrew-phillips-1, I've update the comment from # Mammography only available at level 3 and above. to # Biopsy only available at level 3 and above., is that correct?

@EvaJanouskova I'm not sure what levels biopsy will be done at but that sounds right and yes I agree about removing any mention of mammography

src/tlo/methods/breast_cancer.py

src/tlo/methods/newborn_outcomes.py

src/tlo/methods/contraception.py

src/tlo/methods/healthsystem.py

andrew-phillips-1 · 2023-09-13T08:50:56Z

Hi @EvaJanouskova (and @tbhallett) It's been a while since I worked on the module so I have forgotten all the reading I did at the time. I'm fairly sure that for some of your questions I didn't know the answer to even at that time. When I return I can try to spend some time over the next few weeks trying to read up about it again if that seems something worth me prioritising at this time.

tbhallett · 2023-09-13T08:58:36Z

Hi @EvaJanouskova (and @tbhallett) It's been a while since I worked on the module so I have forgotten all the reading I did at the time. I'm fairly sure that for some of your questions I didn't know the answer to even at that time. When I return I can try to spend some time over the next few weeks trying to read up about it again if that seems something worth me prioritising at this time.

Thanks @andrew-phillips-1. I don't think a wholesale update or anything like that is needed at the moment (although we have something along these lines slated for version 2, when we have someone available to support). For now, we just need to know the equipment that (implicitly) is being used. In the coming weeks, Eva will gather a catalogue of items and ask all disease module leads to identify the ones used in each HSI. This is needed for the costing model and to allow us to look at some analyses that MOH/CHAI are asking about.

andrew-phillips-1 · 2023-09-13T09:13:15Z

Thanks @tbhallett - that seems good and I'm happy to try to do that for the modules I developed. @EvaJanouskova Do you know how other module leads are going about checking on equipment use for their modules ? There are the 2015 clinical guidelines, and I guess the service assessment survey is of some use, but do you know of other sources, that I have perhaps paid insufficient attention to ?

src/tlo/methods/breast_cancer.py

EvaJanouskova · 2023-09-18T16:30:56Z

Great start on this Eva. Only minor comments.

Do we have an idea about how to provide people with a catalogue of equipment they can choose from when making these declarations?

@tbhallett, It depends on the purpose and scope of the catalogue. Are we looking for a simple list for users to reference, where they manually copy equipment names as needed? Should it include only equipment currently modelled, meaning that when a new piece of equipment is added to the model, it must also be added to the catalogue?

What functionalities or requirements on the catalogue did you have in mind?

EvaJanouskova · 2023-09-20T16:13:07Z

Thanks @tbhallett - that seems good and I'm happy to try to do that for the modules I developed. @EvaJanouskova Do you know how other module leads are going about checking on equipment use for their modules ? There are the 2015 clinical guidelines, and I guess the service assessment survey is of some use, but do you know of other sources, that I have perhaps paid insufficient attention to ?

Hi @andrew-phillips-1, I've just received a draft of the new 2023 version of the Malawi standard treatment guidelines. I will make these guidelines and the survey easily available to everyone. Additionally, we will create an equipment catalogue, allowing everyone to find the equipment needed for their module(s) and ensuring consistency by using the same names if used by multiple leaders.

andrew-phillips-1 · 2023-09-20T16:17:45Z

Sounds good @EvaJanouskova

* utils: typo * healthsystem: typo * co: comments about consumable items * healthsyst: comment moved to where it belongs * minor typos * healthsystem: correct argument name arg_service_availability * brc: changes in comments based on discussion with Andrew in #1098 * hs: typos/minor corrections * formatting_demog_data: minor changes is comments * ac: minor comment updates --------- Co-authored-by: Tim Hallett <[email protected]>

matt-graham

Interface generally looks good to me. I've added some further suggested changes and comments below, but the only one I think is vital is around changing HSI_Event._EQUIPMENT from being a class attribute to an instance attribute unless the former was specifically intended.

src/tlo/methods/healthsystem.py

matt-graham · 2024-05-14T16:50:40Z

src/tlo/methods/hsi_event.py

+    _EQUIPMENT: Set[int] = set()  # The set of equipment that is used in the HSI. If any items in this set are not
+    #                               available at the point when the HSI will be run, then the HSI is not run, and the
+    #                               `never_ran` method is called instead. This is a declaration of resource needs, but
+    #                               is private because users are expected to use `add_equipment` to declare equipment
+    #                               needs.
+


I think it would be better to initialise self._EQUIPMENT in the initialiser __init__ method, as setting the value here makes it an attribute of the class rather than a specific instance. That means all instances of the class will share the same _EQUIPMENT set and so equipment added / removed from one instance will affect all other instances. It's possible I'm misunderstanding and that is the desired behaviour though!

Thanks. We absolutely don't want that behaviour. I'm making the change and will also add a test to check that the equipment list for each HSI_Event is kept separate.

src/tlo/methods/hsi_event.py

matt-graham · 2024-05-14T17:08:25Z

tests/test_equipment.py

+    # - using single string for one item descriptor
+    assert eq_default.parse_items('ItemOne')
+    # - using list of strings for item descriptors
+    assert eq_default.parse_items(['ItemOne', 'ItemTwo'])


The assertions here seem to be just that the values returned by parse_items are not 'Falsey' (for example not an empty set). Could we not instead check that we get the expected set of item codes (that is I think {1} and {1, 2} respectively if I'm understanding how parse_items works correctly)?

That's exactly right. I didn't update this from an earlier version of the test file. Next commit will make these tests check for more than 'lack of error and falsiness'

tests/test_equipment.py

… instance and not the class

Co-authored-by: Matt Graham <[email protected]>

…exactly the right resonse, rather than truthiness only.

…m' into EvaJ/equipment/structure_ToRunSim

…rom_multiple_hsi

…quipment/structure_ToRunSim

tbhallett · 2024-05-15T09:22:51Z

bahhhh-- checks failing due to ~~ruff~~ pylint errors on file I haven't touched in this PR.... 🤪

matt-graham · 2024-05-15T09:29:34Z

bahhhh-- checks failing due to ruff errors on file I haven't touched in this PR.... 🤪

I'll take a look - if specific rules are causing false positives I can set them to be ignored.

matt-graham · 2024-05-15T10:00:15Z

bahhhh-- checks failing due to ruff errors on file I haven't touched in this PR.... 🤪

I'll take a look - if specific rules are causing false positives I can set them to be ignored.

Looks like this is due a new pylint release yesterday (version 3.2.0) which added a new possibly-used-before-assignment check as the scheduled checks workflow on master also failed overnight. I'll check whether any of the occurences flagged are actually a problem and either fix or disable the rule.

tbhallett · 2024-05-15T11:04:33Z

bahhhh-- checks failing due to ruff errors on file I haven't touched in this PR.... 🤪

I'll take a look - if specific rules are causing false positives I can set them to be ignored.

Looks like this is due a new pylint release yesterday (version 3.2.0) which added a new possibly-used-before-assignment check as the scheduled checks workflow on master also failed overnight. I'll check whether any of the occurences flagged are actually a problem and either fix or disable the rule.

That makes sense -- thanks very much for this.

matt-graham · 2024-05-15T13:29:30Z

@tbhallett #1349 should fix the pylint errors when running checks on both master and here

matt-graham

Given the failing checks here are not to do with anything in this PR as they are also failing on master and should be fixed by #1349, I would say this is good to go in now.

Edit: A but we won't be able to do without disabling the status check, so maybe we do need to merge in #1349 first.

tbhallett · 2024-05-15T15:17:24Z

~Thanks Matt. I’ll merge it in.~ I'll wait!

EvaJanouskova · 2024-05-23T17:53:18Z

Thank you all for your amazing work! This looks so much better than it ever did. I love it!

Thank you.

EvaJanouskova marked this pull request as draft September 6, 2023 15:50

EvaJanouskova mentioned this pull request Sep 7, 2023

Capturing use of equipment #1078

Open

EvaJanouskova requested a review from tbhallett September 7, 2023 15:50

tbhallett requested changes Sep 12, 2023

View reviewed changes

EvaJanouskova force-pushed the EvaJ/equipment/structure_ToRunSim branch 2 times, most recently from 238459b to c1373e7 Compare September 12, 2023 18:44

andrew-phillips-1 reviewed Sep 16, 2023

View reviewed changes

src/tlo/methods/breast_cancer.py Outdated Show resolved Hide resolved

src/tlo/methods/breast_cancer.py Outdated Show resolved Hide resolved

EvaJanouskova changed the title ~~equipment use~~ equipment use infrastructure Sep 19, 2023

EvaJanouskova changed the title ~~equipment use infrastructure~~ hs: equipment use structure Sep 19, 2023

EvaJanouskova added a commit that referenced this pull request Sep 20, 2023

brc: changes in comments based on discussion with Andrew in #1098

2fbd856

EvaJanouskova force-pushed the EvaJ/equipment/structure_ToRunSim branch 3 times, most recently from ed64e6a to ebe0b6a Compare September 29, 2023 10:09

EvaJanouskova added a commit that referenced this pull request Sep 29, 2023

brc: changes in comments based on discussion with Andrew in #1098

67196fd

EvaJanouskova force-pushed the EvaJ/equipment/structure_ToRunSim branch from 76b9bd6 to f1aa86c Compare October 24, 2023 22:03

EvaJanouskova added a commit that referenced this pull request Oct 25, 2023

brc: changes in comments based on discussion with Andrew in #1098

ce956ae

EvaJanouskova force-pushed the EvaJ/equipment/structure_ToRunSim branch from f1aa86c to 9de2730 Compare November 15, 2023 00:26

This comment was marked as outdated.

Sign in to view

EvaJanouskova force-pushed the EvaJ/equipment/structure_ToRunSim branch from 522c135 to a30017a Compare November 15, 2023 19:01

This comment was marked as outdated.

Sign in to view

EvaJanouskova force-pushed the EvaJ/equipment/structure_ToRunSim branch 2 times, most recently from 5e29244 to 193a81e Compare November 19, 2023 18:23

tbhallett added 3 commits May 13, 2024 21:19

fix typo in test_HealthSystemChangeParameters

fa214a7

linting

3be8363

remove update_availability

959c243

matt-graham requested changes May 14, 2024

View reviewed changes

tbhallett and others added 13 commits May 14, 2024 19:09

use continue rather than nested if/else statment

cc56948

initialise self._EQUIPMENT in the __init__ so that it specific to the…

69e28ca

… instance and not the class

Update src/tlo/methods/hsi_event.py

d340e09

Co-authored-by: Matt Graham <[email protected]>

Update src/tlo/methods/hsi_event.py

45b3b41

Co-authored-by: Matt Graham <[email protected]>

Update src/tlo/methods/hsi_event.py

da5222e

Co-authored-by: Matt Graham <[email protected]>

Update src/tlo/methods/hsi_event.py

e7b40b0

Co-authored-by: Matt Graham <[email protected]>

Update src/tlo/methods/hsi_event.py

204877e

Co-authored-by: Matt Graham <[email protected]>

make checks in test_core_functionality_of_equipment_class test for …

55f9c88

…exactly the right resonse, rather than truthiness only.

Merge remote-tracking branch 'origin/EvaJ/equipment/structure_ToRunSi…

5f52b7b

…m' into EvaJ/equipment/structure_ToRunSim

check logging from multiple HSI_events in test_logging_of_equipment_f…

75e02a9

…rom_multiple_hsi

Merge remote-tracking branch 'refs/remotes/origin/master' into EvaJ/e…

8549403

…quipment/structure_ToRunSim

roll back incidental changes

81b8c58

remove codes_to_items_list.py from this PR

e42e660

matt-graham mentioned this pull request May 15, 2024

Globally disable Pylint E0606 possibly-used-before-assignment rule + fix one case #1349

Merged

matt-graham approved these changes May 15, 2024

View reviewed changes

Merge branch 'master' into EvaJ/equipment/structure_ToRunSim

9370931

tbhallett merged commit 8b0698d into master May 15, 2024
57 checks passed

giordano deleted the EvaJ/equipment/structure_ToRunSim branch May 16, 2024 11:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The `Equipment` Class #1098

The `Equipment` Class #1098

EvaJanouskova commented Sep 6, 2023 •

edited by tbhallett

Loading

tbhallett left a comment

tbhallett Sep 12, 2023

EvaJanouskova Sep 12, 2023

andrew-phillips-1 Sep 16, 2023

EvaJanouskova Sep 18, 2023 •

edited

Loading

andrew-phillips-1 Sep 18, 2023

andrew-phillips-1 commented Sep 13, 2023

tbhallett commented Sep 13, 2023

andrew-phillips-1 commented Sep 13, 2023

EvaJanouskova commented Sep 18, 2023 •

edited

Loading

EvaJanouskova commented Sep 20, 2023

andrew-phillips-1 commented Sep 20, 2023

This comment was marked as outdated.

This comment was marked as outdated.

matt-graham left a comment

matt-graham May 14, 2024

tbhallett May 14, 2024

matt-graham May 14, 2024

tbhallett May 14, 2024

tbhallett commented May 15, 2024 •

edited

Loading

matt-graham commented May 15, 2024

matt-graham commented May 15, 2024

tbhallett commented May 15, 2024

matt-graham commented May 15, 2024

matt-graham left a comment •

edited

Loading

tbhallett commented May 15, 2024 via email •

edited

Loading

EvaJanouskova commented May 23, 2024

The Equipment Class #1098

The Equipment Class #1098

Conversation

EvaJanouskova commented Sep 6, 2023 • edited by tbhallett Loading

TODO

tbhallett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EvaJanouskova Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrew-phillips-1 commented Sep 13, 2023

tbhallett commented Sep 13, 2023

andrew-phillips-1 commented Sep 13, 2023

EvaJanouskova commented Sep 18, 2023 • edited Loading

EvaJanouskova commented Sep 20, 2023

andrew-phillips-1 commented Sep 20, 2023

This comment was marked as outdated.

This comment was marked as outdated.

matt-graham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbhallett commented May 15, 2024 • edited Loading

matt-graham commented May 15, 2024

matt-graham commented May 15, 2024

tbhallett commented May 15, 2024

matt-graham commented May 15, 2024

matt-graham left a comment • edited Loading

Choose a reason for hiding this comment

tbhallett commented May 15, 2024 via email • edited Loading

EvaJanouskova commented May 23, 2024

The `Equipment` Class #1098

The `Equipment` Class #1098

EvaJanouskova commented Sep 6, 2023 •

edited by tbhallett

Loading

EvaJanouskova Sep 18, 2023 •

edited

Loading

EvaJanouskova commented Sep 18, 2023 •

edited

Loading

tbhallett commented May 15, 2024 •

edited

Loading

matt-graham left a comment •

edited

Loading

tbhallett commented May 15, 2024 via email •

edited

Loading