refactor: Consolidate cost loading functions#1567
Merged
Conversation
Contributor
There was a problem hiding this comment.
PR Overview
This PR refactors and consolidates cost loading functions by replacing the old prepare_costs implementation with a unified load_costs function in add_electricity.py, and updates dependent scripts accordingly. Key changes include:
- Changing the load_costs API (renaming parameters, updating logging messages, and improved unit handling)
- Replacing usages in scripts (make_summary.py, make_summary_perfect.py, add_existing_baseyear.py, prepare_sector_network.py) to import and use the new unified function
- Updating the CO2 cost assignments to use the “CO2 intensity” column consistently
Reviewed Changes
| File | Description |
|---|---|
| scripts/add_electricity.py | Refactored load_costs function signature and updated CO2 column mapping |
| scripts/make_summary.py | Replaced prepare_costs with load_costs and adjusted parameter naming |
| scripts/make_summary_perfect.py | Similar updates to cost loading function and parameter names |
| scripts/add_existing_baseyear.py | Updated import and usage of load_costs replacing prepare_costs |
| scripts/prepare_sector_network.py | Removed the old prepare_costs function and updated imports |
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
scripts/add_electricity.py:205
- Multiple duplicate definitions of the load_costs function appear in this file. Consider consolidating them into a single definition to avoid potential conflicts and maintenance issues.
def load_costs(cost_file: str, config: dict, max_hours: dict = None, nyears: float = 1.0) -> pd.DataFrame:
scripts/add_electricity.py:200
- The updated CO2 cost assignment now fetches values from the 'CO2 intensity' column. Please verify that this change aligns with downstream processing, ensuring all references to cost-related CO2 data are updated accordingly.
n.carriers.loc[carriers, "co2_emissions"] = costs.loc[suptechs, "CO2 intensity"].values
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1449 .
Changes proposed in this Pull Request
This pull request focuses on unifying and refactoring cost-related functions across multiple scripts, as well as updating the documentation to reflect these changes. The most important changes include the consolidation of cost functions, updates to the
load_costsfunction, and modifications to various scripts to use the new unified function.Unified the functions
load_costsinadd_electricityandprepare_costsinprepare_sector_networkinto a single functionload_costsinadd_electricity.Refactored the
load_costsfunction inadd_electricity.pyto include detailed docstrings, parameter descriptions, and updated processing logic. [1] [2]Checklist
envs/environment.yaml.config/config.default.yaml.doc/configtables/*.csv.doc/data_sources.rst.doc/release_notes.rstis added.