Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Consolidate cost loading functions #1567

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fneum
Copy link
Member

@fneum fneum commented Mar 5, 2025

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_costs function, and modifications to various scripts to use the new unified function.

  • Unified the functions load_costs in add_electricity and prepare_costs in prepare_sector_network into a single function load_costs in add_electricity.

  • Refactored the load_costs function in add_electricity.py to include detailed docstrings, parameter descriptions, and updated processing logic. [1] [2]

Checklist

  • I tested my contribution locally and it works as intended.
  • Code and workflow changes are sufficiently documented.
  • Changed dependencies are added to envs/environment.yaml.
  • Changes in configuration options are added in config/config.default.yaml.
  • Changes in configuration options are documented in doc/configtables/*.csv.
  • Sources of newly added data are documented in doc/data_sources.rst.
  • A release note doc/release_notes.rst is added.

@fneum fneum requested a review from lisazeyen as a code owner March 5, 2025 14:00
@fneum fneum requested a review from Copilot March 5, 2025 14:00

Choose a reason for hiding this comment

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

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
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.

Consolidate cost data loading
1 participant