Cleaning up modify_prenetwork

[toniseibold]

Changes proposed in this Pull Request

Calling int(snakemake.wildcards.planning_horizons multiple times in the rule modify_prenetwork.
Using the convention of planning_horizon and passing it to functions as input instead of defining internal variables over and over again.

I needed to reuse the rule in a different workflow and wanted to hard code the planning horizon which is now much easier/cleaner to do.

Checklist

Required:

• Changes are tested locally and behave as expected.
• Code and workflow changes are documented. not applicable
• A brief description of the changes has been added to Changelog.md. not applicable

If applicable:

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

[Copilot]

Pull request overview

This PR aims to reduce repeated conversions of snakemake.wildcards.planning_horizons by introducing a single planning_horizon variable and passing it into helper functions in modify_prenetwork.py, aligning function signatures to a consistent pattern.

Changes:

• Updated multiple helper functions to accept planning_horizon as an explicit argument instead of re-parsing the Snakemake wildcard.
• Updated some call sites to pass planning_horizon through (e.g., first_technology_occurrence, add_wasserstoff_kernnetz, must_run, etc.).
• Modified __main__ mock defaults (clusters/run) used for local execution.

Comments suppressed due to low confidence (2)

scripts/pypsa-de/modify_prenetwork.py:1436

• new_boiler_ban now requires planning_horizon, but the main block still calls it without the new argument. This will raise a TypeError when running the rule; pass planning_horizon here (and keep the call sites consistent with the updated function signature).

    modify_mobility_demand(n, snakemake.input.modified_mobility_data)

    new_boiler_ban(n)

    fix_new_boiler_profiles(n)

scripts/pypsa-de/modify_prenetwork.py:1449

• unravel_carbonaceous_fuels now takes planning_horizon, but the call site still uses the old signature. This will crash at runtime; pass planning_horizon into the function call.

    unravel_carbonaceous_fuels(n)

    unravel_gasbus(n, costs)

---

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

[lindnemi]

@copilot apply changes based on the low confidence comments in this thread

[Copilot]

@copilot apply changes based on the low confidence comments in this thread

Fixed in commit 55c63a4 — both new_boiler_ban and unravel_carbonaceous_fuels now receive planning_horizon as the second argument.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[pworschischek-aggmag]

Hello @lindnemi, I have a small refactoring request for modify_prenetwork.py: We are about to figure out clean paths for model extensions in PyPSA-AT. We have solutions for Snakemake extensions without touching upstream .smk files and hooking into the additional_functionality.py feature without touching the PyPSA-DE file while preserving everythin thats done there.
The olny piece missing are upstream scripts such as pypsa-de/modify_prenetwork.py. The reason is that we cannot import everything the script does. The scripts logic is under the __main__ guard and in script scope. Python cannot reuse that from another entry point.

A possible solution is putting all buisness logic in pypsa-de/modify_prenetwork.py inside a new orchestrator function (often named main()). Another script, e.g. scripts/pypsa-at/modifications.py could then import the orechestrator function from schripts/pypsa-de/ to extend the script. In Snakemake, a pattern like below uses the AT-script whch runs everything from upstream DE.

# rules/pypsa-at/solve.smk  ← AT-governed, edit freely
use rule upstream_rule as upstream_rule_at with:
    input:
        **rules.upstream_rule.input,
        at_specific_data=resources("at_specific_data.csv"),  # AT-specific stuff on top 
    script:
        scirpts("pypsa-at/modifications.py")  # import and runs the DE orcehstrator 

ruleorder: upstream_rule_at > upstream_rule

Besides the downstream model extensibility, there are other reasons why this might be a good idea:

• keeps the global scope tidy: Any variable in the scripts global scope is automatically available in all functions defined in that script. This is not obvios to new Python users and can cause bug and unexpected behavior.
• obviate function signatures: The example below cannot happen anymore

def new_boiler_ban(n):  # uses snakemake from global scope
    year = int(snakemake.wildcards.planning_horizons)  
    for ct in snakemake.params.fossil_boiler_ban:
         # ...

You may want to give it a thought. It would help us over at PyPSA-AT, but its not super critical right now. I just thought it might also be valuable input for PyPSA-Eur where more soft-forks exist and extendability is more of an issue.

PS: the cherry on top would be a module without dash '-' in its path and with an __init__.py file that exposes a public stable API with all orechstrator functions from the pypsa-de model layer (aka Python code in scripts/pypsa-de) =)