Merge pull request #250 from Climate-REF/fix-esmvaltool-paths

Author: lewisjared

.github/workflows/ci-integration.yaml

@@ -40,8 +40,9 @@ jobs:
           key: ilamb
       - name: Run tests
         run: |
+          make virtual-environment
           make fetch-test-data
-          uv run ref datasets fetch-data ilamb
+          uv run ref datasets fetch-data --registry ilamb
           make test
       # Upload the scratch and results directories as artifacts
       - name: Upload scratch artifacts

changelog/250.fix.md

@@ -0,0 +1,2 @@
+Fixed how the path to ESMValTool outputs was determined,
+and added support for outputs in nested directories.

docs/how-to-guides/running-metrics-locally.py

@@ -94,7 +94,7 @@
     data_catalog={
         SourceDatasetType.CMIP6: data_catalog,
     },
-    metric=provider.get("global-mean-timeseries"),
+    metric=metric,
     provider=provider,
 )
 
@@ -144,6 +144,7 @@
 # This will not perform and validation/verification of the output results.
 
 # %%
+definition.output_directory.mkdir(parents=True, exist_ok=True)
 direct_result = metric.run(definition=definition)
 assert direct_result.successful
 

packages/ref-core/src/cmip_ref_core/logging.py

@@ -112,8 +112,14 @@ def redirect_logs(definition: MetricExecutionDefinition, log_level: str) -> Gene
         The logger will also be reset to this level after leaving the context manager.
 
     """
+    app_logger_configured = hasattr(logger, "default_handler_id")
+
     # Remove existing default log handler
-    remove_log_handler()
+    # This swallows the logs from the app logger
+    # If the app logger hasn't been configured yet, we don't need to remove it,
+    # as logs will also be written to the console as loguru adds a stderr handler by default
+    if app_logger_configured:
+        remove_log_handler()
 
     # Add a new log handler for the execution log
     output_file = definition.output_directory / EXECUTION_LOG_FILENAME
@@ -131,7 +137,10 @@ def redirect_logs(definition: MetricExecutionDefinition, log_level: str) -> Gene
 
         # Reset the logger to the default
         logger.remove(file_handler_id)
-        add_log_handler(**logger.default_handler_kwargs)  # type: ignore[attr-defined]
+
+        # We only re-add the app handler if it was configured before
+        if app_logger_configured:
+            add_log_handler(**logger.default_handler_kwargs)  # type: ignore[attr-defined]
 
 
 __all__ = ["add_log_handler", "capture_logging", "logger", "redirect_logs"]

packages/ref-core/tests/unit/test_logging.py

@@ -52,6 +52,31 @@ def test_redirect_logs(definition, caplog):
     assert text_inner in output_file.read_text()
 
 
+def test_redirect_logs_without_handler(definition):
+    definition, output_file = definition
+
+    assert not hasattr(logger, "default_handler_id")
+    with redirect_logs(definition, "INFO"):
+        logger.info("inner")
+    assert not hasattr(logger, "default_handler_id")
+
+    assert "inner" in output_file.read_text()
+
+
+def test_redirect_logs_stdlog_captured(definition):
+    definition, output_file = definition
+
+    with redirect_logs(definition, "INFO"):
+        import logging
+
+        stdlog = logging.getLogger("stdlog")
+        stdlog.info("stdlog inner")
+        stdlog.debug("stdlog debug")
+
+    assert "stdlog inner" in output_file.read_text()
+    assert "stdlog debug" not in output_file.read_text()
+
+
 def test_redirect_logs_exception(definition, caplog):
     log_level = "DEBUG"
 

packages/ref-metrics-esmvaltool/src/cmip_ref_metrics_esmvaltool/metrics/base.py

@@ -142,7 +142,7 @@ def build_metric_result(
             metadata = yaml.load(metadata_file.read_text(encoding="utf-8"))
             for filename in metadata:
                 caption = metadata[filename].get("caption", "")
-                relative_path = Path(filename).relative_to(result_dir.absolute())
+                relative_path = definition.as_relative_path(filename)
                 if relative_path.suffix in plot_suffixes:
                     key = OutputCV.PLOTS.value
                 else:

packages/ref-metrics-esmvaltool/tests/integration/test_metrics.py

@@ -0,0 +1,34 @@
+import pytest
+from cmip_ref_metrics_esmvaltool import provider
+
+from cmip_ref.models import MetricExecutionResult as MetricExecutionResultModel
+from cmip_ref.solver import solve_metric_executions
+from cmip_ref.testing import validate_result
+from cmip_ref_core.metrics import Metric
+
+metrics = [pytest.param(metric, id=metric.slug) for metric in provider.metrics()]
+
+
+@pytest.mark.slow
+@pytest.mark.parametrize("metric", metrics)
+def test_metrics(metric: Metric, data_catalog, tmp_path, config, mocker):
+    mocker.patch.object(MetricExecutionResultModel, "metric_execution_group")
+    # Ensure the conda prefix is set
+    provider.configure(config)
+
+    # Get the first match from the data catalog
+    execution = next(
+        solve_metric_executions(
+            data_catalog=data_catalog,
+            metric=metric,
+            provider=provider,
+        )
+    )
+
+    # Run the metric
+    definition = execution.build_metric_execution_info(output_root=config.paths.scratch)
+    definition.output_directory.mkdir(parents=True, exist_ok=True)
+    result = metric.run(definition)
+
+    # Check the result
+    validate_result(config, result)

packages/ref-metrics-pmp/tests/integration/test_variability_modes.py

@@ -2,13 +2,9 @@
 from cmip_ref_metrics_pmp import provider
 from cmip_ref_metrics_pmp.variability_modes import ExtratropicalModesOfVariability
 
-from cmip_ref.database import Database
-from cmip_ref.executor import handle_execution_result
 from cmip_ref.models import MetricExecutionResult as MetricExecutionResultModel
 from cmip_ref.solver import solve_metric_executions
-from cmip_ref_core.metrics import MetricExecutionResult
-from cmip_ref_core.pycmec.metric import CMECMetric
-from cmip_ref_core.pycmec.output import CMECOutput
+from cmip_ref.testing import validate_result
 
 variability_metrics = [
     pytest.param(metric, id=metric.slug)
@@ -17,31 +13,6 @@
 ]
 
 
-def validate_result(config, result: MetricExecutionResult):
-    database = Database.from_config(config)
-    metric_execution_result = MetricExecutionResultModel(
-        metric_execution_group_id=1,
-        dataset_hash=result.definition.metric_dataset.hash,
-        output_fragment=str(result.definition.output_fragment()),
-    )
-    database.session.add(metric_execution_result)
-    database.session.flush()
-
-    assert result.successful
-
-    # Validate bundles
-    CMECMetric.load_from_json(result.to_output_path(result.metric_bundle_filename))
-    CMECOutput.load_from_json(result.to_output_path(result.output_bundle_filename))
-
-    # Create a fake log file
-    result.to_output_path("out.log").touch()
-
-    # This checks if the bundles are valid
-    handle_execution_result(
-        config, database=database, metric_execution_result=metric_execution_result, result=result
-    )
-
-
 @pytest.mark.slow
 @pytest.mark.parametrize("metric", variability_metrics)
 def test_variability_modes(metric: ExtratropicalModesOfVariability, data_catalog, tmp_path, config, mocker):

packages/ref/src/cmip_ref/executor/__init__.py

@@ -104,10 +104,10 @@ def _copy_file_to_results(
     if not (input_directory / filename).exists():
         raise FileNotFoundError(f"Could not find {filename} in {input_directory}")
 
-    if not output_directory.exists():
-        output_directory.mkdir(parents=True)
+    output_filename = output_directory / filename
+    output_filename.parent.mkdir(parents=True, exist_ok=True)
 
-    shutil.copy(input_directory / filename, output_directory / filename)
+    shutil.copy(input_directory / filename, output_filename)
 
 
 def handle_execution_result(

packages/ref/src/cmip_ref/solver.py

@@ -97,6 +97,9 @@ def build_metric_execution_info(self, output_root: pathlib.Path) -> MetricExecut
         """
         Build the metric execution info for the current metric execution
         """
+        # Ensure that the output root is always an absolute path
+        output_root = output_root.resolve()
+
         # This is the desired path relative to the output directory
         fragment = pathlib.Path() / self.provider.slug / self.metric.slug / self.metric_dataset.hash