search

UCL / TLOmodel

Epidemiology modelling framework for the Thanzi la Onse project
https://www.tlomodel.org/
MIT License
13 stars 5 forks source link

Refactor setting of `PROPERTIES` columns across `Module`s #1429

Closed willGraham01 closed 3 months ago

willGraham01 commented 4 months ago

Concerns #1428 |

Large portion of the additions in the diff are the PROPERTIES attributes of the modules being linted by tox check and split over multiple lines, so apologies for that to any reviewers.

Key changes introduced are within the core module though - specifically the new default_value that Propertys now take and the abstractly implemented initialise_population method. Changes to disease modules should mostly be deletions of the explicit setting of columns and addition of default values for categories where necessary.

tamuri commented 3 months ago

I suggest breaking this PR into several PRs. One adding the new functionality, the others refactoring specific modules to use the new functionality. This does mean that we end up supporting both old and new approaches for a time, but it is easier to work with modellers to update and check their modules and also identify any regressions easily.

willGraham01 commented 3 months ago

I suggest breaking this PR into several PRs. One adding the new functionality, the others refactoring specific modules to use the new functionality. This does mean that we end up supporting both old and new approaches for a time, but it is easier to work with modellers to update and check their modules and also identify any regressions easily.

No problem, it should just be a case of cherry-picking the various commits from this branch (they're on a per-module basis). Will get started on this.

willGraham01 commented 3 months ago

Closing in favour of (the first of many) starting with #1436. Leaving the branch here for cherry-picking purposes.