Skip to content

Use geo_boundaries modules as source of spatial shapefiles - #2252

Open
brynpickering wants to merge 7 commits into
PyPSA:masterfrom
open-energy-transition:refactor/modelblock-regions
Open

Use geo_boundaries modules as source of spatial shapefiles#2252
brynpickering wants to merge 7 commits into
PyPSA:masterfrom
open-energy-transition:refactor/modelblock-regions

Conversation

@brynpickering

Copy link
Copy Markdown
Contributor

Proof of concept for moving our processes to using modelblocks.

I've kept it light touch, leaving the existing build_shapes rule to take the consolidated shapefile and explode it out into its respective outputs.

Comparing results

In the following, I've plotted the current shapes in blue with thin black lines and then overlaid the new shapes with 50% opacity red shapes and black dotted lines. Differences are then evident where the colour changes or there are line mismatches when zooming in.

NUTS

They are identical for NUTS minus a few river tributaries (e.g. southern NLD / north BEL):

river_overlap

Non-NUTS

The boundaries are slightly off (e.g. here in BIH):

BIH_mismatch

The geoboundaries open data also comes predominantly from OSM although for BIH it comes from wikipedia, which explains the difference. Kosovo comes from OSM and it has the same region shapes.

EEZ

Looks the same:
EEZ

Handling modules

Since this is the first PR using a modelblock, I have made some opinionated decisions on how to handle them:

  1. The configs exist separately, each referencing the their own schema in the file. Overriding the default config requires defining items under the config key for the module in the module section of the base config.
  • Maybe this should be called overrides?
  • thoughts on how to handle this more gracefully @irmcodebase?
  1. The rules exist in a separate module subdirectory. Since it's only one rule per module, it could all sit in a single modules.smk file, I'm just unsure how long that file will get if we add lots of modules.
  2. I've gone lightest touch on running the module, by letting it get shapes for all countries even when only requesting a subset in the config.
  3. requires using --use-conda in the snakemake call to ensure the geo_boundaries conda environment is installed. This avoids needing to add their deps to our own.
    • there are plans to make pixi envs possible here which might make things a bit smoother, with a common dependency cache
  4. Since the module config can be overridden per pypsa-eur scenario, I've left the resources pathvar as resources. However, it is mostly download data, which would be better off having in data. Currently, we can't split this as two separate pathvars, but I've upstreamed the request.

Geo-boundaries design decisions

  • Pull in all countries and then only filter on build_shapes. This add bloat but could only be avoided by doing some config magic to filter out keys under the geo_boundaries scenarios config key before passing the config over to the module.
  • Use geoBoundaries for non-nuts data
  • Do not split EEZs using the module's voronoi method.

Checklist

Required:

  • Changes are tested locally and behave as expected.
  • Code and workflow changes are documented.
  • A release note entry is added to doc/release_notes.md.
  • The description is human-written and any AI-generated content is marked.

If applicable:

  • Changes in configuration options are reflected in scripts/lib/validation.
  • For new data sources or versions, these instructions have been followed.
  • New rules are documented in the appropriate doc/*.md files.

Copilot AI review requested due to automatic review settings August 3, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces the geo_boundaries modelblock module as the primary source for spatial boundary inputs (land + maritime), and removes the legacy pipeline that retrieved/constructed boundaries from EU NUTS 2021, MarineRegions EEZ, and OSM Overpass-derived ADM1 sources.

Changes:

  • Integrate modelblocks-org/module_geo_boundaries via Snakemake modules and wire its parquet output into build_shapes.
  • Replace shape building inputs to consume the module’s consolidated land/maritime dataset and derive offshore shapes via dissolve.
  • Remove legacy retrieval/build rules and dataset version entries for eu_nuts2021, eez, and osm_boundaries; extend config validation/schema to support a new modules section.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Snakefile Bumps Snakemake min version; injects module config expansion and includes geo_boundaries module rules.
scripts/retrieve_osm_boundaries.py Removes Overpass-based OSM ADM1 boundary retrieval script.
scripts/build_osm_boundaries.py Removes OSM ADM1 geometry construction script.
scripts/build_shapes.py Switches onshore/offshore shape construction to consume geo_boundaries parquet output.
scripts/_helpers.py Adds add_module_config() to merge module default configs with user overrides.
scripts/lib/validation/config/modules.py Adds pydantic models for the new modules config section.
scripts/lib/validation/config/_schema.py Adds modules to the top-level validated config schema.
scripts/lib/validation/config/data.py Removes data-source config entries for datasets no longer used (eez/eu_nuts2021/osm_boundaries).
rules/modules/geo_boundaries.smk Defines the Snakemake module import and namespacing for geo_boundaries.
rules/build_electricity.smk Updates build_shapes inputs to use module-produced parquet instead of legacy sources.
rules/retrieve.smk Removes retrieval rules for eez, eu_nuts2021, and osm_boundaries.
rules/common.smk Adds an import (currently unused).
data/versions.csv Removes dataset version records for removed shape-related legacy datasets.
config/config.default.yaml Adds default modules.geo_boundaries configuration; removes legacy dataset configs.
config/modules/geo_boundaries.yaml Adds module default configuration (scenario/country selection, EEZ voronoi disabled).
config/schema.default.json Updates generated schema to include new modules config and remove legacy dataset configs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/build_shapes.py Outdated
Comment thread rules/common.smk
Copilot AI review requested due to automatic review settings August 3, 2026 11:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (1)

scripts/_helpers.py:1178

  • add_module_config will currently raise a low-level KeyError / FileNotFoundError (and potentially pass None into update_config) before validate_config() has a chance to produce a user-friendly validation error. Adding a small guard around missing modules, missing default_config files, and empty YAML makes failures easier to diagnose.
    for module_config in config["modules"].values():
        default_config = yaml.safe_load(
            Path(module_config["default_config"]).read_text()
        )
        update_config(default_config, module_config["config"])

Copilot AI review requested due to automatic review settings August 3, 2026 11:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/_helpers.py:1178

  • add_module_config currently iterates over config["modules"].values() and reads each default_config without any module context or error handling. If a module is misconfigured (missing keys, config: null, or a missing file), the resulting KeyError/FileNotFoundError will be hard to diagnose because the module name/path won’t be included in the exception.

Consider iterating over items() (so you keep the module name), defaulting overrides to {} when absent, and wrapping the file read to raise an error that clearly identifies the offending module and path.

    for module_config in config["modules"].values():
        default_config = yaml.safe_load(
            Path(module_config["default_config"]).read_text()
        )
        update_config(default_config, module_config["config"])

rules/build_electricity.smk:143

  • build_shapes now depends on a hard-coded geo_boundaries scenario output (resources("shapes/pypsa-eur.parquet")). This couples the workflow to the module config’s scenario key/name; if someone renames the module scenario (or wants multiple module scenarios), Snakemake will fail with a missing input file.

To make this more robust, consider centralising the scenario name in one place (e.g., a constant exported from rules/modules/geo_boundaries.smk, or a dedicated config key) and referencing that here, instead of duplicating the literal string.

rule build_shapes:
    input:
        shapes=resources("shapes/pypsa-eur.parquet"),
        nuts3_gdp=rules.retrieve_jrc_ardeco.output["ardeco_gdp"],
        nuts3_pop=rules.retrieve_jrc_ardeco.output["ardeco_pop"],

Copilot AI review requested due to automatic review settings August 3, 2026 12:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Snakefile:41

  • add_module_config(config) mutates config["modules"][...]["config"] from “overrides” into the fully-resolved module config. As a result, scenario validation currently deep-copies the already-mutated config and only applies scenario_overrides, so scenario-specific changes to modules.*.default_config (and a clean re-resolution of module config) will not take effect during validation and can lead to inconsistent per-scenario module configuration.

Consider keeping an unmodified raw_config copy, and for each scenario: merge overrides into raw_config, then call add_module_config(merged) before validate_config(merged).

add_module_config(config)

validate_config(config)

run = config["run"]

scripts/_helpers.py:1178

  • add_module_config assumes config["modules"] exists and that each module entry contains a dict config override and a readable default_config file. Because this runs before validate_config, misconfigurations (e.g. modules: null, config: null, missing file) will currently raise low-level exceptions (TypeError, KeyError, FileNotFoundError) instead of a clear, actionable message.

Adding defensive checks and normalizing yaml.safe_load(... ) to {} will make failures much easier to diagnose.

    for module_config in config["modules"].values():
        default_config = yaml.safe_load(
            Path(module_config["default_config"]).read_text()
        )
        update_config(default_config, module_config["config"])

Copilot AI review requested due to automatic review settings August 3, 2026 12:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (2)

scripts/_helpers.py:1178

  • add_module_config reads and merges module configs before config validation, and assumes config["modules"] exists and that module_config["config"] is always a dict. If a user sets modules: {…} to null/missing, or sets config: null, this will raise a KeyError/TypeError/FileNotFoundError with a low-level traceback instead of a clear validation error. Consider making this helper robust (treat missing/None overrides as {} and raise a module-scoped error if default_config can't be read).
    for module_config in config["modules"].values():
        default_config = yaml.safe_load(
            Path(module_config["default_config"]).read_text()
        )
        update_config(default_config, module_config["config"])
        module_config["config"] = default_config

scripts/lib/validation/config/modules.py:42

  • ModulesConfig.geo_boundaries uses Field(default=_ModuleConfig(...)), which is a mutable default model instance created at import time. This is inconsistent with the rest of the config schema (which uses default_factory=...) and risks shared-state surprises if the default instance is ever mutated (directly or via .model_dump()/copies). Prefer default_factory here to generate a fresh _ModuleConfig per validation.
    geo_boundaries: _ModuleConfig = Field(
        default=_ModuleConfig(
            default_config=Path("config/modules/geo_boundaries.yaml"),
            version="v1.0.1",
        ),
        description="Configuration for the geo_boundaries module.",

Copilot AI review requested due to automatic review settings August 3, 2026 14:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (3)

scripts/lib/validation/config/modules.py:42

  • ModulesConfig.geo_boundaries uses default=_ModuleConfig(...), which instantiates and validates a FilePath at import time. Because the default path is relative ("config/modules/geo_boundaries.yaml"), importing this module can fail when the current working directory is not the project root (e.g. schema generation, tooling, or tests executed from another directory). Use default_factory to defer instantiation/validation until model creation (consistent with other config models like DataConfig.version_files).
    geo_boundaries: _ModuleConfig = Field(
        default=_ModuleConfig(
            default_config=Path("config/modules/geo_boundaries.yaml"),
            version="v1.0.1",
        ),

scripts/build_shapes.py:321

  • non_nuts_countries is a NumPy array of strings; using .any() relies on element truthiness and is less explicit than checking array size/length. This makes the intent harder to read and can behave unexpectedly with missing values. Prefer an explicit emptiness check.
    if non_nuts_countries.any():

scripts/_helpers.py:1178

  • add_module_config reads module_config["default_config"] via Path(...).read_text() relative to the current working directory. This can break when the workflow is executed from a different directory (e.g. via Snakemake --directory) even though configs are documented as project-relative. Consider resolving relative paths against the project root and guarding against missing modules to avoid a KeyError.
    for module_config in config["modules"].values():
        default_config = yaml.safe_load(
            Path(module_config["default_config"]).read_text()
        )

@fneum
fneum requested a review from bobbyxng August 3, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants