Improve download rule structure and logging

tests/conftest.py

@@ -1,5 +1,6 @@
 """Shared test fixtures."""
 
+import os
 import shutil
 import zipfile
 from pathlib import Path
@@ -10,15 +11,34 @@
 TEST_FILES = "https://zenodo.org/records/19401947/files/test_suite.zip?download=1"
 
 
-@pytest.fixture(scope="module")
-def module_path():
-    """Parent directory of the project."""
-    # If your module needs files in resources/user/, place automated downloads here.
-    return Path(__file__).parent.parent
+def dir_to_dir_copy(source_dir: Path, target_dir: Path) -> None:
+    """Copy all files from one directory into another."""
+    target_dir.mkdir(parents=True, exist_ok=True)
+    for file in source_dir.iterdir():
+        if file.is_file():
+            shutil.copy(file, target_dir / file.name)
+
+
+@pytest.fixture(scope="session")
+def cds_api_file() -> Path:
+    """Ensure the CDS API file is present in the system."""
+    cds_file = Path.home().joinpath(".cdsapirc")
+
+    if not cds_file.exists():
+        cdsapi_key = os.getenv("CDSAPI_KEY")
+
+        if cdsapi_key:
+            cds_file.write_text(
+                f"url: https://cds.climate.copernicus.eu/api\nkey: {cdsapi_key}\n"
+            )
+        else:
+            raise RuntimeError("Neither CDSAPI_KEY or the CDS API file is present.")
+
+    return cds_file
 
 
 @pytest.fixture(scope="session")
-def user_path() -> Path:
+def user_path(cds_api_file) -> Path:
     """Download and unzip test files."""
     user_dir = Path("resources/user/")
     # If test suite has been downloaded, assume everything is OK.
@@ -31,3 +51,26 @@ def user_path() -> Path:
         with zipfile.ZipFile(test_zip, "r") as zfile:
             zfile.extractall(user_dir)
     return user_dir
+
+
+@pytest.fixture(scope="module")
+def module_path(user_path: Path):
+    """Parent directory of the project."""
+    module_path = Path(__file__).parent.parent
+
+    integration_dir = Path(module_path / "tests/integration")
+
+    # clean files produced by the integration test
+    if integration_dir.exists():
+        shutil.rmtree(integration_dir / "resources/", ignore_errors=True)
+        shutil.rmtree(integration_dir / "results/", ignore_errors=True)
+
+    # copy user inputs needed by the module
+    dir_to_dir_copy(user_path / "MNE", integration_dir / "resources/inputs/MNE")
+    # copy downloads with unstable Github CI access
+    dir_to_dir_copy(
+        module_path / "tests/files/hydrobasins",
+        integration_dir / "resources/module/resources/automatic/hydrobasins",
+    )
+
+    return module_path

tests/files/README.md

@@ -0,0 +1,5 @@
+# Synthetic tests files
+
+Used during integration CI tests.
+
+- `hydrobasins`: synthetic alternative to the `.zip` files downloaded from the HydroBASINS website.

tests/integration_test.py

@@ -4,34 +4,13 @@
 Contents may be updated in future template updates.
 """
 
-import os
-import shutil
 import subprocess
 import tomllib
 from pathlib import Path
 
 import pytest
 from clio_tools.data_module import ModuleInterface
 
-CDSAPI_KEY = os.getenv("CDSAPI_KEY")
-CDS_FILE = Path.home().joinpath(".cdsapirc")
-
-
-@pytest.fixture(scope="module")
-def integration_path(user_path: Path, module_path: Path):
-    """Ensures the minimal integration test is ready."""
-    integration_dir = Path(module_path / "tests/integration")
-    if integration_dir.exists():  # clean everything
-        shutil.rmtree(integration_dir / "resources/", ignore_errors=True)
-        shutil.rmtree(integration_dir / "results/", ignore_errors=True)
-    user_integ_dir = integration_dir / "resources/inputs/"
-    files_to_copy = ["MNE/powerplants.parquet", "MNE/shapes.parquet"]
-    for file in files_to_copy:
-        destination_file = Path(user_integ_dir / file)
-        destination_file.parent.mkdir(parents=True, exist_ok=True)
-        shutil.copy(user_path / file, destination_file)
-    return integration_dir
-
 
 @pytest.fixture(scope="module")
 def pixi_platforms(module_path) -> list[str]:
@@ -95,17 +74,11 @@ def test_snakemake_all_failure(module_path):
     assert "INVALID" in str(process.stderr)
 
 
-@pytest.mark.skipif(
-    not (CDSAPI_KEY or CDS_FILE.exists()),
-    reason="Neither CDSAPI_KEY env var nor ~/.cdsapirc file available.",
-)
-def test_snakemake_integration_testing(integration_path):
+def test_snakemake_integration_testing(module_path):
     """Run a light-weight test simulating someone using this module."""
-    if CDSAPI_KEY and not CDS_FILE.exists():
-        CDS_FILE.write_text(
-            f"url: https://cds.climate.copernicus.eu/api\nkey: {CDSAPI_KEY}\n"
-        )
-
     assert subprocess.run(
-        "snakemake --use-conda --cores 1", shell=True, check=True, cwd=integration_path
+        "snakemake --use-conda --cores 1",
+        shell=True,
+        check=True,
+        cwd=module_path / "tests/integration",
     )

workflow/Snakefile

@@ -32,7 +32,7 @@ with open(workflow.source_path("internal/settings.yaml"), "r") as f:
 
 # Add all your includes here.
 include: "rules/_utils.smk"
-include: "rules/automatic.smk"
+include: "rules/downloads.smk"
 include: "rules/statistics.smk"
 include: "rules/basins.smk"
 include: "rules/powerplants.smk"

workflow/rules/_utils.smk

@@ -1,5 +1,20 @@
 """Utility functions for the module."""
 
+from snakemake.io import apply_wildcards
+
+
+def prefer_existing(file_path):
+    """Prevents re-downloads even in forced cases.
+
+    Useful for protecting invariable files.
+    """
+
+    def resolve_output(wildcards):
+        path = apply_wildcards(str(file_path), wildcards)
+        return before_update(path) if exists(path) else path
+
+    return resolve_output
+
 
 def validate_year_config():
     start = config["years"]["start"]

workflow/rules/basins.smk

@@ -3,7 +3,7 @@
 
 rule basins_extract_pfafstetter_level:
     input:
-        zip_file=rules.download_basin.output.path,
+        zip_file=prefer_existing(rules.download_basin.output.path),
     output:
         parquet_file=temp(
             "<resources>/automatic/hydrobasins/{continent}_{level}.parquet"

workflow/rules/automatic.smk → workflow/rules/downloads.smk

@@ -14,7 +14,7 @@ rule download_eia:
     message:
         "Download the EIA International energy statistics in bulk."
     shell:
-        r'curl -fsSLo {output.zipfile:q} "{params.url}"'
+        'curl -fsSLo {output.zipfile:q} "{params.url}" 2>{log:q}'
 
 
 rule download_basin:
@@ -34,7 +34,7 @@ rule download_basin:
     message:
         "Downloading HydroBASINS file for '{wildcards.continent}'."
     shell:
-        r'curl -fsSLo {output.path:q} "{params.url}"'
+        'curl -fsSLo {output.path:q} "{params.url}" 2>{log:q}'
 
 
 rule download_cutout:

workflow/scripts/basins_extract_pfafstetter_level.py

@@ -7,7 +7,6 @@
 
 if TYPE_CHECKING:
     snakemake: Any
-sys.stderr = open(snakemake.log[0], "w", buffering=1)
 SHP_ZIP_PATH = "hybas_{continent}_lev{level}_v1c.shp"
 
 
@@ -36,6 +35,7 @@ def basins_extract_pfafstetter_level(zip_file, continent, level, parquet_file):
 
 
 if __name__ == "__main__":
+    sys.stderr = open(snakemake.log[0], "w", buffering=1)
     basins_extract_pfafstetter_level(
         zip_file=snakemake.input.zip_file,
         continent=snakemake.params.continent,

workflow/scripts/powerplants_get_inflow_m3.py

@@ -10,7 +10,6 @@
 
 if TYPE_CHECKING:
     snakemake: Any
-sys.stderr = open(snakemake.log[0], "w", buffering=1)
 
 
 def powerplants_get_inflow_m3(
@@ -66,6 +65,7 @@ def powerplants_get_inflow_m3(
 
 
 if __name__ == "__main__":
+    sys.stderr = open(snakemake.log[0], "w", buffering=1)
     powerplants_get_inflow_m3(
         shapes_file=snakemake.input.shapes,
         powerplants_file=snakemake.input.adjusted_powerplants,

workflow/scripts/powerplants_get_inflow_mwh.py

@@ -12,7 +12,6 @@
 
 if TYPE_CHECKING:
     snakemake: Any
-sys.stderr = open(snakemake.log[0], "w", buffering=1)
 
 
 # ---
@@ -162,6 +161,7 @@ def powerplants_get_inflow_mwh(
 
 
 if __name__ == "__main__":
+    sys.stderr = open(snakemake.log[0], "w", buffering=1)
     powerplants_get_inflow_mwh(
         inflow_m3_file=snakemake.input.inflow_m3,
         powerplants_file=snakemake.input.adjusted_powerplants,