Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving to a single registry and importing on request not automatically #323

Merged
Changes from 1 commit
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
b70875e
Draft registry and schema modules for a single MODULE_REGISTRY
davidorme Sep 29, 2023
59f174e
Temporary switch to conceal all the existing autodiscovery code
davidorme Sep 29, 2023
f1b0b0b
Added register_module steps to Config.build_schema, switch to MODULE_…
davidorme Sep 29, 2023
05a3331
register_module needs to _import_ the module before the components ca…
davidorme Sep 29, 2023
d4e9273
select_models in vr_run now uses MODULE_REGISTRY
davidorme Sep 29, 2023
5933633
Updated core.constants to MODULE_REGISTRY and moved it to core.consta…
davidorme Sep 29, 2023
957aeff
Stripped schema stuff from old home in config
davidorme Sep 29, 2023
ada8cf6
Fix typing on ValidatorWithDefaults
davidorme Sep 29, 2023
560ff0a
Stripped model registry stuff from old home in core.base_model, remov…
davidorme Sep 29, 2023
35cee84
Updated core.registry.register_constants_classes to handle absence of…
davidorme Sep 29, 2023
7c2b5ce
Removing model registration code from within model.__init__ files
davidorme Sep 29, 2023
688f5b9
Removing old autodiscovery code
davidorme Sep 29, 2023
42fb9eb
Stripping references to old SCHEMA_, MODEL_ and CONSTANTS_REGISTRY
davidorme Sep 29, 2023
e935405
Simplifying schema path modification and updating from dpath.utils to…
davidorme Oct 2, 2023
f13eb68
Clearer comments
davidorme Oct 2, 2023
3fcb089
Cleaner return code
davidorme Oct 2, 2023
791087c
New ConstantsDataclass, updated load_constants
davidorme Oct 3, 2023
4d4cbca
Updating constants classes to use ConstantsDataclass
davidorme Oct 3, 2023
d462f7a
Restructure of registry to use ModuleInfo and simplify to single func…
davidorme Oct 3, 2023
f5b9953
Removing temporary _new suffixes on new registry structures
davidorme Oct 3, 2023
8602305
Updating Config.build_schema to import the required modules, leaving …
davidorme Oct 3, 2023
11cb6c4
Restored module registration location within module __init__.py, upda…
davidorme Oct 3, 2023
7bdf4a9
Updating main to use updated registry
davidorme Oct 3, 2023
f2b69b7
Reversed core module registration in core.__init__ - use register_mod…
davidorme Oct 3, 2023
15b285d
First tweak of tests to new MODULE_REGISTRY
davidorme Oct 3, 2023
077de52
Tweaks to registry constants_class checking
davidorme Oct 4, 2023
f9b1cf9
Removing BaseModel types, annotating where those _should_ be used
davidorme Oct 4, 2023
45b82a5
Initial core constants dataclass
davidorme Oct 4, 2023
34076b5
Longer variable names
davidorme Oct 4, 2023
cbd2fa6
Tidying in registry
davidorme Oct 4, 2023
84974c4
Tidying in schema
davidorme Oct 4, 2023
6cbdf7a
Reinstate list not generator for traversing schema properties
davidorme Oct 4, 2023
abb63b4
Explicitly try and import constants modules, rather than relying on t…
davidorme Oct 4, 2023
4ec8a4c
Note another BaseModel typing fixme
davidorme Oct 4, 2023
0fa336e
standardise module schema filenames across core and models on module-…
davidorme Oct 4, 2023
6b53d7e
constants loader update
davidorme Oct 4, 2023
4eb3652
Docstring updates
davidorme Oct 4, 2023
e92cbe4
Updating mypy in testing and pre-commit
davidorme Oct 4, 2023
d15d777
Updating defining_new_models
davidorme Oct 5, 2023
adf439c
Outdated poetry.lock
davidorme Oct 5, 2023
1a4890b
Minor fixes from code review
davidorme Oct 5, 2023
cdcd5d5
Added universal, non-configurable constants
davidorme Oct 5, 2023
a370838
Merge branch 'develop' into 278-importing-virtual_rainforest-package-…
davidorme Oct 5, 2023
e8468e6
Added draft configurable constant to core.CoreConsts and updated schema
davidorme Oct 6, 2023
53ff5bc
Testing for constants loader and test driven development of load_cons…
davidorme Oct 6, 2023
9284d2a
Tests for ConstantsDataclass and minor edits to docstrings and logging
davidorme Oct 6, 2023
abfa72a
Registry and register_module testing
davidorme Oct 6, 2023
03b6295
Merge branch '278-importing-virtual_rainforest-package-always-causes-…
davidorme Oct 7, 2023
516990e
Update of select_models and updating test to use MODULE_REGISTRY
davidorme Oct 9, 2023
6e721f4
Trapped constants_loader issue where module is missing from config
davidorme Oct 9, 2023
3758f9c
Updating configure_models and testing
davidorme Oct 9, 2023
1ef9b3a
More updating of tests in test_main.py
davidorme Oct 9, 2023
dc78b53
Removed old MODEL_REGISTRY testing from test_base_model.py
davidorme Oct 9, 2023
454d24b
Removing outdated register_schema testing
davidorme Oct 10, 2023
ea35ab6
Updating test_data.py to replace use of MODEL_REGISTRY
davidorme Oct 10, 2023
02d8ced
Added log_check functionality to check a subset of the caplog.records…
davidorme Oct 10, 2023
90b658e
Revised workflow for register_module
davidorme Oct 10, 2023
ef7cdfa
Moving back to explicit module registration, not registration on import
davidorme Oct 12, 2023
38a43c7
Updated Config.build_schema to use register_module again
davidorme Oct 12, 2023
41f341b
Updating test_registry suite, adding ModuleInfo.is_core
davidorme Oct 12, 2023
62cbbcb
Renaming TestModel in test_modules - these are not pytest classes, bu…
davidorme Oct 12, 2023
4a89828
Fixing test_config for new registration process
davidorme Oct 12, 2023
78e7737
Fixing test_schema for new registration process
davidorme Oct 12, 2023
6c9315b
Updating test_abiotic_simple_model
davidorme Oct 12, 2023
0705fdf
Moving imports inside tests in test_abiotic_simple.py
davidorme Oct 12, 2023
73ec9a2
Updated test_hydrology_model.py
davidorme Oct 12, 2023
2141e7a
More robust testing of model init properties
davidorme Oct 12, 2023
1297094
Updated test_litter_models.py
davidorme Oct 12, 2023
a6077f6
Updated test_soil_models.py
davidorme Oct 12, 2023
cf5a41c
test_main fixes and minor changes
davidorme Oct 12, 2023
b34553e
Turning on autoclear of MODULE_REGISTRY to enforce correct per test r…
davidorme Oct 12, 2023
1fa69d8
Doc build fixes
davidorme Oct 12, 2023
3ed36f6
Updating definining_new_models.md
davidorme Oct 12, 2023
6e2d949
Merge branch 'develop' into 278-importing-virtual_rainforest-package-…
davidorme Oct 12, 2023
04536cc
nitpick ignore odd JSONSchema docstring issue
davidorme Oct 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updating test_registry suite, adding ModuleInfo.is_core
davidorme committed Oct 12, 2023
commit 41f341b0d142c619a73e222253416b27b38c2f73
2 changes: 0 additions & 2 deletions tests/core/test_module/__init__.py

This file was deleted.

3 changes: 3 additions & 0 deletions tests/core/test_modules/bad_name/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"""A test module providing a single model as expected."""

from tests.core.test_modules.bad_name.test_model import TestModel # noqa: F401
13 changes: 13 additions & 0 deletions tests/core/test_modules/bad_name/test_model.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""A test module to test module registration."""

from virtual_rainforest.core.base_model import BaseModel


class TestModel(BaseModel):
"""A test module."""

model_name = "name_is_not_bad_name"
required_init_vars = tuple()
lower_bound_on_time_scale = "1 day"
upper_bound_on_time_scale = "1 month"
vars_updated = tuple()
4 changes: 4 additions & 0 deletions tests/core/test_modules/no_model/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"""A test module providing no model.

Or indeed anything else, but the model is checked first by register_module.
"""
3 changes: 3 additions & 0 deletions tests/core/test_modules/one_model/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
"""A test module providing a single model as expected."""

from tests.core.test_modules.one_model.test_model import TestModel # noqa: F401
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"type": "object",
"properties": {
"test_module": {}
"one_model": {}
},
"required": [
"test_module"
"one_model"
]
}
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@
class TestModel(BaseModel):
"""A test module."""

model_name = "name"
model_name = "one_model"
required_init_vars = tuple()
lower_bound_on_time_scale = "1 day"
upper_bound_on_time_scale = "1 month"
6 changes: 6 additions & 0 deletions tests/core/test_modules/two_models/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""A test module providing a single model as expected."""

from tests.core.test_modules.two_models.test_model import ( # noqa: F401
TestModel1,
TestModel2,
)
23 changes: 23 additions & 0 deletions tests/core/test_modules/two_models/test_model.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
"""A test module to test module registration."""

from virtual_rainforest.core.base_model import BaseModel


class TestModel1(BaseModel):
"""A test module."""

model_name = "two_models"
required_init_vars = tuple()
lower_bound_on_time_scale = "1 day"
upper_bound_on_time_scale = "1 month"
vars_updated = tuple()


class TestModel2(BaseModel):
"""A second unwanted test module."""

model_name = "two_models"
required_init_vars = tuple()
lower_bound_on_time_scale = "1 day"
upper_bound_on_time_scale = "1 month"
vars_updated = tuple()
112 changes: 66 additions & 46 deletions tests/core/test_registry.py
Original file line number Diff line number Diff line change
@@ -9,110 +9,130 @@


@pytest.mark.parametrize(
argnames="module_name, model_name, raises, exp_log",
argnames="module_name, raises, exp_log",
argvalues=[
pytest.param(
"virtual_rainforest.core",
None,
does_not_raise(),
(
(INFO, "Registering virtual_rainforest.core module components"),
(INFO, "Schema registered for module core:"),
(INFO, "Constants class registered for module core: CoreConsts"),
(INFO, "Registering module: virtual_rainforest.core"),
(INFO, "Schema registered for virtual_rainforest.core:"),
(
INFO,
"Constants class registered for "
"virtual_rainforest.core: CoreConsts",
),
),
id="core_import_good",
),
pytest.param(
"tests.core.test_module",
"test_module",
"tests.core.test_modules.one_model",
does_not_raise(),
(
(INFO, "Registering tests.core.test_module module components"),
(INFO, "Registering model class for test_module model: TestModel"),
(INFO, "Schema registered for module test_module:"),
(INFO, "Constants class registered for module test_module: TestConsts"),
(INFO, "Registering module: tests.core.test_modules.one_model"),
(
INFO,
"Registering model class for "
"tests.core.test_modules.one_model: TestModel",
),
(INFO, "Schema registered for tests.core.test_modules.one_model:"),
(
INFO,
"Constants class registered for "
"tests.core.test_modules.one_model: TestConsts",
),
),
id="model_import_good",
),
pytest.param(
"tests.core.test_modelo",
"test_modelo",
"tests.core.test_modules.nothing_here",
pytest.raises(ModuleNotFoundError),
(
(
CRITICAL,
"Unknown module - registration failed: tests.core.test_modelo",
"Unknown module - registration failed: "
"tests.core.test_modules.nothing_here",
),
),
id="model_import_bad_module",
),
pytest.param(
"virtual_rainforest.core",
"test_modelo",
"tests.core.test_modules.no_model",
pytest.raises(RuntimeError),
(
(INFO, "Registering virtual_rainforest.core module components"),
(CRITICAL, "No model should be registered for the core module"),
(
CRITICAL,
"Model object not found in tests.core.test_modules.no_model",
),
),
id="core_import_with_model",
id="model_import_no_model",
),
pytest.param(
"tests.core.test_module",
None,
"tests.core.test_modules.two_models",
pytest.raises(RuntimeError),
(
(INFO, "Registering tests.core.test_module module components"),
(CRITICAL, "A model class is required to register model modules"),
(
CRITICAL,
"More than one model defined in tests.core.test_modules.two_models",
),
),
id="model_import_without_model",
id="model_import_two_models",
),
pytest.param(
"tests.core.test_module",
"test_modelo",
pytest.param( # TODO - may become redundant if the name is set automatically.
"tests.core.test_modules.bad_name",
pytest.raises(RuntimeError),
(
(INFO, "Registering tests.core.test_module module components"),
(INFO, "Registering module: tests.core.test_modules.bad_name"),
(
CRITICAL,
"Different model_name attribute and module name "
"in tests.core.test_module",
"in tests.core.test_modules.bad_name",
),
),
id="model_import_bad_name",
),
],
)
def test_registry(caplog, module_name, model_name, raises, exp_log):
def test_registry(caplog, module_name, raises, exp_log):
"""Test the registry loading.

This uses a dummy model to impersonate the plant model, because importing any real
models triggers `register_module` calls from the module __init__.py files.
"""

from tests.core.test_module.test_model import TestModel
from virtual_rainforest.core.registry import MODULE_REGISTRY, register_module
from virtual_rainforest.core.base_model import BaseModel
from virtual_rainforest.core.constants_class import ConstantsDataclass
from virtual_rainforest.core.registry import (
MODULE_REGISTRY,
ModuleInfo,
register_module,
)

MODULE_REGISTRY.clear()

# Get the short name
_, _, short_name = module_name.rpartition(".")

# Either do not provide a model (core) or provide a dummy with a name set by the
# test
if model_name is None:
model = None
else:
setattr(TestModel, "model_name", model_name)
model = TestModel

caplog.clear()

with raises:
register_module(module_name=module_name, model=model)
register_module(module_name=module_name)

if isinstance(raises, does_not_raise):
# Slightly random selection of checks
# Test the detailed structure of the registry for the module
assert short_name in MODULE_REGISTRY
assert MODULE_REGISTRY[short_name].model is model
assert isinstance(MODULE_REGISTRY[short_name].schema, dict)
assert isinstance(MODULE_REGISTRY[short_name].constants_classes, dict)
mod_info = MODULE_REGISTRY[short_name]
assert isinstance(mod_info, ModuleInfo)

if not mod_info.is_core:
assert issubclass(mod_info.model, BaseModel)

assert isinstance(mod_info.schema, dict)
assert isinstance(mod_info.constants_classes, dict)
for c_class in mod_info.constants_classes.values():
assert issubclass(c_class, ConstantsDataclass)

log_check(caplog=caplog, expected_log=exp_log)
# Check the last N entries in the log match the expectation.
log_check(
caplog=caplog, expected_log=exp_log, subset=slice(-len(exp_log), None, None)
)
21 changes: 10 additions & 11 deletions virtual_rainforest/core/registry.py
Original file line number Diff line number Diff line change
@@ -43,6 +43,9 @@ class ModuleInfo:
constants_classes: dict[str, ConstantsDataclass]
"""A dictionary of module constants classes. The individual ConstantsDataclass
objects are keyed by their name."""
is_core: bool
"""Logical flag indicating if an instance contains registration information for the
core module."""


MODULE_REGISTRY: dict[str, ModuleInfo] = {}
@@ -87,7 +90,7 @@ def register_module(module_name: str) -> None:
_, _, module_name_short = module_name.rpartition(".")

if module_name_short in MODULE_REGISTRY:
LOGGER.warning(f"Module already registered: {module_name_short}")
LOGGER.warning(f"Module already registered: {module_name}")
return

# Try and import the module from the name to get a reference to the module
@@ -99,7 +102,7 @@ def register_module(module_name: str) -> None:

is_core = module_name == "virtual_rainforest.core"

LOGGER.info(f"Registering {module_name} module components")
LOGGER.info(f"Registering module: {module_name}")

# Locate _one_ BaseModel class in the module root if this is not the core.
if is_core:
@@ -133,9 +136,7 @@ def register_module(module_name: str) -> None:
raise RuntimeError(msg)

# Register the resulting single model class
LOGGER.info(
f"Registering model class for {module_name_short} model: {model.__name__}"
)
LOGGER.info(f"Registering model class for {module_name}: {model.__name__}")

# Register the schema
with resources.as_file(
@@ -151,9 +152,7 @@ def register_module(module_name: str) -> None:
)
raise excep

LOGGER.info(
"Schema registered for module %s: %s ", module_name_short, schema_file_path
)
LOGGER.info("Schema registered for %s: %s ", module_name, schema_file_path)

# Find and register the constant dataclasses
try:
@@ -176,11 +175,11 @@ def register_module(module_name: str) -> None:

for class_name in constants_classes.keys():
LOGGER.info(
"Constants class registered for module %s: %s ",
module_name_short,
"Constants class registered for %s: %s ",
module_name,
class_name,
)

MODULE_REGISTRY[module_name_short] = ModuleInfo(
model=model, schema=schema, constants_classes=constants_classes
model=model, schema=schema, constants_classes=constants_classes, is_core=is_core
)