Type hints generally incorrect

[mikesmit]

Many type hints are incorrect in the current version of the source code (mostly in simulation.py). This makes it harder to identify and fix issues like #53

[mikesmit]

I'm using this comment to track "bug or feature" questions I run into trying to correctly assign types

• I'm confused by the "baseline" input
  • What is this actually for? it's optional and the example doesn't set it and the object we build with it is called "Reform" but also there is an actual reform....
  • Also later... we call _apply_region_to_simulation on both the reform and the baseline, but we only ever use the reform in that method (confusingly this has now be reassigned to a Reform object although it was originally the dict | None from input).
  • I think this is a bug and it should be baseline for baseline and reform for reform, but not clear. either way we
• baseline refers to three things and at times appears to have three different types of values (reform has two only because we also have "reformed" instead of "reform")
  • there is an attribute baseline of type dict which.. appears to be intended to reflect the dict | None passed in as a construction parameter
  • there is a second, overlapping attribute also called baseline which is a country simulation (I.e. the initialized simulation associated with the baseline)
  • but we also assign the overloaded attribute to a Reform object which also implies the original parameter can be an int (rather than just dict | none)
• "selected"
  • Why do we only set "selected" if this is a reform (also need to fix to be "| None" to reflect this). Surely if there is no reform we just make selected the baseline?
  • Similarly, why it is typed as the microsimulations instead of simulation... but it can be either right? (I can have a reform comparison for a household sim)
• the "time_period" attribute
  • is non optional but its assigned to the parameter which is defaulted to none (so inferring optional)
  • but also is then assigned to default_time_period of the simulation which .. can't be none, but also doesn't have to be passed to the constructor.
  • TLDR... I think optional and if it's none just don't set it on the simulation...
• _apply_region_to_simulation silently does nothing unless you are in the US (and some other constraints) - is that right? It seems like it should fail if it can't apply the region (but maybe I misunderstand how the region stuff works)

[mikesmit]

Sleeping on it, I believe the actual clear bugs are the following:

1. baseline overloaded - baseline is defined twice to mean two things. the second eclipses the first.
   2. the baseline country simulation
   3. the reform used to build the baseline country simulation4
2. _apply_region_to_simulation applies the wrong reform to baseline - instead of applying the baseline reform (as when it initializes the simulation first) it applies the reform reform
3. "selected" should default to baseline if there is no reform

[mikesmit]

@nikhilwoodruff I pushed a branch with the update I have (commit log discusses the limitations) here: https://github.com/PolicyEngine/policyengine.py/tree/55_fix_type_hints