Pass through updated appointment footprint

[matt-graham]

Fixes #291.

Changes HSI_Event.run to return the updated appointment footprint, if any, returned by the apply method of the event.

To get the tests to pass I also needed to change the logic in HealthSystemScheduler.apply slightly to only recompute the squeeze factors when an updated appointment footprint is returned if HealthSystem.mode_appt_constraints is non-zero. While the the squeeze factors are initially set to zero in this case this previously was not guaranteed to be the case on the recomputation following an updated footprint, however this wasn't previously causing a test failure as the updated footprint was not being passed through and so this code path was never active.

[matt-graham]

The test_labour test failure seems to be as the apply method of an HSI_Labour_ReceivesSkilledBirthAttendanceDuringLabour event is returning an appointment footprint Counter({'NormalDelivery': 0}) which is found to be non-valid as it uses a zero value (with appt_footprint_is_valid requiring all values are positive). This updated appointment footprint appears to be getting generated by these lines

TLOmodel/src/tlo/methods/labour.py

Lines 2909 to 2913 in 5f82456

	if df.at[person_id, 'la_sepsis'] or (df.at[person_id, 'la_antepartum_haem'] != 'none') or \
	df.at[person_id, 'la_obstructed_labour'] or df.at[person_id, 'la_uterine_rupture'] \
	or df.at[person_id, 'ps_htn_disorders'] == 'eclampsia' \
	or df.at[person_id, 'ps_htn_disorders'] == 'severe_pre_eclamp':
	actual_appt_footprint['NormalDelivery'] = actual_appt_footprint['CompDelivery'] # todo: is this right?

which I think may not be doing what was intended. @joehcollins @tbhallett was the intention to change from an appointment footprint requiring CompDelivery to one instead requiring NormalDelivery?

[matt-graham]

As the other test failures are all in pregnancy related test modules (test_pregnancy_supervisor, test_contraception, test_newborn_outcomes, test_postnatal) it seems plausible these are all due to the same underlying issue.

[tbhallett]

The test_labour test failure seems to be as the apply method of an HSI_Labour_ReceivesSkilledBirthAttendanceDuringLabour event is returning an appointment footprint Counter({'NormalDelivery': 0}) which is found to be non-valid as it uses a zero value (with appt_footprint_is_valid requiring all values are positive). This updated appointment footprint appears to be getting generated by these lines

TLOmodel/src/tlo/methods/labour.py

Lines 2909 to 2913 in 5f82456

	if df.at[person_id, 'la_sepsis'] or (df.at[person_id, 'la_antepartum_haem'] != 'none') or \
	df.at[person_id, 'la_obstructed_labour'] or df.at[person_id, 'la_uterine_rupture'] \
	or df.at[person_id, 'ps_htn_disorders'] == 'eclampsia' \
	or df.at[person_id, 'ps_htn_disorders'] == 'severe_pre_eclamp':
	actual_appt_footprint['NormalDelivery'] = actual_appt_footprint['CompDelivery'] # todo: is this right?

which I think may not be doing what was intended. @joehcollins @tbhallett was the intention to change from an appointment footprint requiring CompDelivery to one instead requiring NormalDelivery?

I wil defer to @joehcollins on this, but I would guess that the line within the long if clause (all the things that might make the delivery complicated rather than normal), should instead just be
return self.make_appt_footprint({'CompDelivery': 1})

[joehcollins]

Hi @tbhallett @matt-graham, yes sorry thats a very old bit of code which was intending to do exactly what Tim describes above. Very happy for you to change.

[matt-graham]

Hi @tbhallett @matt-graham, yes sorry thats a very old bit of code which was intending to do exactly what Tim describes above. Very happy for you to change.

Thanks @joehcollins, I'll update this now.

[matt-graham]

The change in labour.py seems to have fixed the all the previously failing tests. @tbhallett anything else you think needs changing before merging?

[tbhallett]

The change in labour.py seems to have fixed the all the previously failing tests. @tbhallett anything else you think needs changing before merging?

All looks good to me