Refactor BaseModel

[davidorme]

Description

This implements @dalonsoa's suggested refactor of BaseModel from #363. We thought this seemed like a good idea at the Github meeting so I've worked it through.

Briefly, that changes how we set model class attributes from:

class NewModel(BaseModel):
     model_name="base_model"
     ...

to

class NewModel(
     BaseModel,
     model_name="base_model"
):
     ...

Another change that fell out of this update is that it looked much cleaner to have a single model_update_bounds attribute set like ('1 day', '1 month') rather than separating those as lower_bound_on_time_scale and upper_bound_on_time_scale. It means _check_time_bounds_units is replaced by _check_model_update_bounds, which is only called once and also moves the test that 1 day < 1 month inside the check function.

Pros on the switch

• The BaseModel code becomes much simpler. Before we had a bunch of properties that might or might not be set on a new model and had to test whether they'd been set or not and then what they were. Now they are explicitly set when __init__subclass__ runs.
• Model definition becomes more robust. Now these values are arguments to the class creation signature, users get all the usual warnings about forgotten arguments and typing.
• The testing code becomes much cleaner. Before, we were using exec() to run chunks of text that described model definitions in order to set different attributes for testing. We can now just pass those attributes in to tests as kwargs to the model creation call.

Cons on the switch

• We now don't have an obvious place to put model specific docstrings on the attribute. For example, I had a docstring on PlantsModel.required_init_vars that gave an overview. I've moved that into a specific section in the model docstring instead. (Honestly I'm not sure this is really a con - that seems like a more logical place for people to read about model requirements).

Fixes #363 (issue)

Type of change

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change which fixes an issue)

Key checklist

• Make sure you've run the pre-commit checks: $ pre-commit run -a
• All tests pass: $ poetry run pytest

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dd41f74) 93.50% compared to head (c1d9d8d) 93.30%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #365      +/-   ##
===========================================
- Coverage    93.50%   93.30%   -0.20%     
===========================================
  Files           59       59              
  Lines         2954     2866      -88     
===========================================
- Hits          2762     2674      -88     
  Misses         192      192              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dalonsoa]

What a great idea XDDD

Jokes aside, the code now looks much cleaner and I feel it will be more robust. Great work!

[jacobcook1995]

LGTM!

[vgro]

LGTM :-)