From 41513ea563f760608fbbd0839d96b50d6b0f89ad Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Thu, 26 Feb 2026 15:38:36 +1100 Subject: [PATCH 1/5] refactor(config): add BaseConfig.render hook Add render() method to BaseConfig that delegates to cookiecutter renderer. This enables configs to override rendering behavior while maintaining backward compatibility with existing template-based rendering. --- src/rompy/core/config.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/rompy/core/config.py b/src/rompy/core/config.py index 438b172..391efa1 100644 --- a/src/rompy/core/config.py +++ b/src/rompy/core/config.py @@ -44,3 +44,23 @@ class BaseConfig(RompyBaseModel): # noop call for config objects def __call__(self, *args, **kwargs): return self + + def render(self, context: dict, output_dir: Path | str) -> str: + """Render the configuration template to the output directory. + + This method orchestrates the template rendering process. The default implementation + uses cookiecutter rendering with the template and checkout defined on this config. + Subclasses can override this method to implement alternative rendering strategies + (e.g., direct file writing, Jinja2, custom logic). + + Args: + context: Full context dictionary. Expected to contain at least 'runtime' and 'config' keys. + output_dir: Target directory for rendered output. + + Returns: + str: Path to the staging directory (workspace) containing rendered files. + """ + # Import locally to avoid potential circular imports at module import time + from rompy.core.render import render as cookiecutter_render + + return cookiecutter_render(context, self.template, output_dir, self.checkout) From 937038aa3c14524149478fdf1fcd10b01346b134 Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Thu, 26 Feb 2026 15:38:38 +1100 Subject: [PATCH 2/5] test(modelrun): assert generate delegates to config.render Add unit test verifying ModelRun.generate() calls config.render() with correct arguments. Test uses mock to intercept render() and validate delegation behavior. --- tests/test_config_render_delegation.py | 46 ++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 tests/test_config_render_delegation.py diff --git a/tests/test_config_render_delegation.py b/tests/test_config_render_delegation.py new file mode 100644 index 0000000..c83bbea --- /dev/null +++ b/tests/test_config_render_delegation.py @@ -0,0 +1,46 @@ +import unittest.mock + +from rompy.model import ModelRun +from rompy.core.config import BaseConfig + + +def test_modelrun_delegates_to_config_render(tmp_path): + """ModelRun.generate() delegates rendering to config.render().""" + + # Setup + class TestConfig(BaseConfig): + def render(self, context: dict, output_dir): + # real implementation replaced by mock in the test + return str(tmp_path / "staging") + + config = TestConfig(template="../rompy/templates/base", checkout=None) + model_run = ModelRun(run_id="test", output_dir=str(tmp_path), config=config) + + # Patch the class method so the instance call is intercepted. Inspect + # positional args to find the context and output_dir regardless of whether + # the mock was bound to the instance or the class. + # Patch by import path to avoid binding issues with Pydantic-generated classes + with unittest.mock.patch.object( + TestConfig, "render", return_value=str(tmp_path / "staging") + ) as mock_render: + # Execute + result = model_run.generate() + + # Verify delegation + mock_render.assert_called_once() + call_args = mock_render.call_args + pos_args = call_args[0] + + # Locate the context dict (contains 'runtime') and the output_dir arg + context_arg = next( + (a for a in pos_args if isinstance(a, dict) and "runtime" in a), None + ) + output_dir_arg = next( + (a for a in pos_args if str(a) == str(model_run.output_dir)), None + ) + + assert isinstance(context_arg, dict) + assert "runtime" in context_arg + assert "config" in context_arg + assert output_dir_arg == model_run.output_dir + assert result == str(tmp_path / "staging") From 329a1c7072dfad0dd0cbca57c597c88b1d497156 Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Thu, 26 Feb 2026 15:38:40 +1100 Subject: [PATCH 3/5] refactor(modelrun): delegate rendering to config.render Update ModelRun.generate() to call config.render() instead of directly calling rompy.core.render.render(). This decouples rendering logic from ModelRun and allows configs to customize rendering behavior. --- src/rompy/model.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/rompy/model.py b/src/rompy/model.py index 787bbec..370d37c 100644 --- a/src/rompy/model.py +++ b/src/rompy/model.py @@ -18,7 +18,6 @@ from rompy.backends import BackendConfig from rompy.backends.config import BaseBackendConfig from rompy.core.config import BaseConfig -from rompy.core.render import render from rompy.core.time import TimeRange from rompy.core.types import RompyBaseModel from rompy.logging import get_logger @@ -256,9 +255,15 @@ def generate(self) -> str: # Render templates logger.info(f"Rendering model templates to {self.output_dir}/{self.run_id}...") - staging_dir = render( - cc_full, self.config.template, self.output_dir, self.config.checkout - ) + # Delegate rendering to the config object's render() method when available. + # Use getattr to avoid attribute errors for callable configs that may not + # implement render(). Fall back to the cookiecutter render implementation + # for older config objects. + # Delegate rendering to the config object's render() method. This enforces + # that config objects provide a render() implementation and allows tests + # to patch/spy on that method without falling back to the cookiecutter + # implementation. + staging_dir = self.config.render(cc_full, self.output_dir) logger.info("") # Use the log_box utility function @@ -366,9 +371,7 @@ def run(self, backend: BackendConfig, workspace_dir: Optional[str] = None) -> bo # Pass the config object and workspace_dir to the backend return backend_instance.run(self, config=backend, workspace_dir=workspace_dir) - def postprocess( - self, processor: "BasePostprocessorConfig", **kwargs - ) -> Dict[str, Any]: + def postprocess(self, processor, **kwargs) -> Dict[str, Any]: """ Postprocess the model outputs using the specified processor configuration. From 1ba3a84048551ea2184beacd26c3e6066eb78815 Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Thu, 26 Feb 2026 16:17:05 +1100 Subject: [PATCH 4/5] Remove comments --- src/rompy/model.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/rompy/model.py b/src/rompy/model.py index 370d37c..243eca0 100644 --- a/src/rompy/model.py +++ b/src/rompy/model.py @@ -254,15 +254,9 @@ def generate(self) -> str: cc_full["config"] = self.config # Render templates - logger.info(f"Rendering model templates to {self.output_dir}/{self.run_id}...") - # Delegate rendering to the config object's render() method when available. - # Use getattr to avoid attribute errors for callable configs that may not - # implement render(). Fall back to the cookiecutter render implementation - # for older config objects. - # Delegate rendering to the config object's render() method. This enforces - # that config objects provide a render() implementation and allows tests - # to patch/spy on that method without falling back to the cookiecutter - # implementation. + logger.info( + f"Rendering model configurations to {self.output_dir}/{self.run_id}..." + ) staging_dir = self.config.render(cc_full, self.output_dir) logger.info("") From 8767aa74b916cad2d9c299e1e61d3b7df5bff2c0 Mon Sep 17 00:00:00 2001 From: Tom Durrant Date: Thu, 26 Feb 2026 16:27:47 +1100 Subject: [PATCH 5/5] fix: update render() to return None, fix f-string syntax and variable references --- src/rompy/core/config.py | 4 ++-- src/rompy/model.py | 10 ++++------ tests/test_config_render_delegation.py | 6 +++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/rompy/core/config.py b/src/rompy/core/config.py index 391efa1..5d5c54e 100644 --- a/src/rompy/core/config.py +++ b/src/rompy/core/config.py @@ -45,7 +45,7 @@ class BaseConfig(RompyBaseModel): def __call__(self, *args, **kwargs): return self - def render(self, context: dict, output_dir: Path | str) -> str: + def render(self, context: dict, output_dir: Path | str): """Render the configuration template to the output directory. This method orchestrates the template rendering process. The default implementation @@ -63,4 +63,4 @@ def render(self, context: dict, output_dir: Path | str) -> str: # Import locally to avoid potential circular imports at module import time from rompy.core.render import render as cookiecutter_render - return cookiecutter_render(context, self.template, output_dir, self.checkout) + cookiecutter_render(context, self.template, output_dir, self.checkout) diff --git a/src/rompy/model.py b/src/rompy/model.py index 243eca0..84dcb08 100644 --- a/src/rompy/model.py +++ b/src/rompy/model.py @@ -254,10 +254,8 @@ def generate(self) -> str: cc_full["config"] = self.config # Render templates - logger.info( - f"Rendering model configurations to {self.output_dir}/{self.run_id}..." - ) - staging_dir = self.config.render(cc_full, self.output_dir) + logger.info(f"Rendering model configurations to {self.staging_dir}...") + self.config.render(cc_full, self.output_dir) logger.info("") # Use the log_box utility function @@ -268,8 +266,8 @@ def generate(self) -> str: logger=logger, add_empty_line=False, ) - logger.info(f"Model files generated at: {staging_dir}") - return staging_dir + logger.info(f"Model files generated at: {self.staging_dir}") + return self.staging_dir def zip(self) -> str: """Zip the input files for the model run diff --git a/tests/test_config_render_delegation.py b/tests/test_config_render_delegation.py index c83bbea..af51967 100644 --- a/tests/test_config_render_delegation.py +++ b/tests/test_config_render_delegation.py @@ -11,7 +11,7 @@ def test_modelrun_delegates_to_config_render(tmp_path): class TestConfig(BaseConfig): def render(self, context: dict, output_dir): # real implementation replaced by mock in the test - return str(tmp_path / "staging") + pass config = TestConfig(template="../rompy/templates/base", checkout=None) model_run = ModelRun(run_id="test", output_dir=str(tmp_path), config=config) @@ -21,7 +21,7 @@ def render(self, context: dict, output_dir): # the mock was bound to the instance or the class. # Patch by import path to avoid binding issues with Pydantic-generated classes with unittest.mock.patch.object( - TestConfig, "render", return_value=str(tmp_path / "staging") + TestConfig, "render", return_value=None ) as mock_render: # Execute result = model_run.generate() @@ -43,4 +43,4 @@ def render(self, context: dict, output_dir): assert "runtime" in context_arg assert "config" in context_arg assert output_dir_arg == model_run.output_dir - assert result == str(tmp_path / "staging") + assert str(result) == str(tmp_path / "test")