1368 disturbance registry

[dalonsoa]

Description

Adds the disturbance model discovery process. I have refactor the existing discovery engine such that it works equally for normal models and disturbance models. A disturbance testing model has been added for testing purposes within a disturbance folder. As the same model discovery process is used, the very same rules in terms of module name and class name conventions apply to the disturbances. New tests have been limited to just the new functionality, as the logic of the registry and discovery has already been thoroughly tested for normal models.

The priority attribute will be added in the final PR, when this is hooked into the main workflow and an order needs to be defined for the disturbance models.

Fixes #1368

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
• Relevant documentation reviewed and updated

[codecov-commenter]

Codecov Report

❌ Patch coverage is 81.92771% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.55%. Comparing base (42d5a01) to head (0249e9a).

Files with missing lines	Patch %	Lines
virtual_ecosystem/core/base_model.py	81.81%	6 Missing ⚠️
...s/disturbance_testing/disturbance_testing_model.py	66.66%	6 Missing ⚠️
virtual_ecosystem/core/configuration.py	72.72%	3 Missing ⚠️

Additional details and impacted files

@@                    Coverage Diff                     @@
##           1367_disturbance_model    #1391      +/-   ##
==========================================================
- Coverage                   94.70%   94.55%   -0.16%     
==========================================================
  Files                          71       73       +2     
  Lines                        7503     7568      +65     
==========================================================
+ Hits                         7106     7156      +50     
- Misses                        397      412      +15     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jacobcook1995]

This all looks sensible to me, I won't approve it though as I'm aware that this is an area where @davidorme has much stronger opinions than me and Taran

[davidorme]

This all looks sensible to me, I won't approve it though as I'm aware that this is an area where @davidorme has much stronger opinions than me and Taran

Ha. There's a lot written into that bolded much 😄

[davidorme]

Neat. As you say, makes good re-use of the existing discovery mechanisms. One question about typing!

[davidorme]

I think what is going on here is that we'd "ideally" have this as:

def _discover_models(models: ModuleType, of_type: type[BaseModel] | type[BaseDisturbance]) ->  list[type[BaseModel] | type[BaseDisturbance)]:

But because we can't import those two classes outside of the functions because of circularity (which bugs me but 🤷 ) we instead use this to tell the function that it is generically getting a type of something (not an instance etc.).

Is that right - if so, could you add a comment to that effect? It's probably pretty entry level but right now generic typing is out of my comfort zone, so it would be useful to have an explanation to hand 😄 .

Of course, if my explanation isn't right, then it needs a comment even more!

[dalonsoa]

Actually, no. We do want type[T] - i.e. to use generics - because that way I am locking the types of the inputs and outputs together. It's like saying that whatever the input, the output is a list if those same things. In your example, from the typing perspective there is no relationship between input and output. I could have type[BaseModel] as input and have a list of type[BaseDisturbance] as outputs.

Generics are really powerful, but can also be quite confusing.

[davidorme]

Got it. Thanks.

[dalonsoa]

Docstring expanded with some details.

[TaranRallings]

Looks sensible to me as well.