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

Update discharge emission factors and overrides CH #6260

Merged
merged 3 commits into from
Dec 18, 2023

Conversation

pierresegonne
Copy link
Member

@pierresegonne pierresegonne commented Dec 18, 2023

Issue

Description

Update discharge emission factors and overrides following the update of the unknown emission factor in #6032

Preview

Double check

  • I have tested my parser changes locally with poetry run test_parser "zone_key"
  • I have run pnpx prettier --write . and poetry run format to format my changes.

@github-actions github-actions bot added the zone config Pull request or issue for zone configurations label Dec 18, 2023
Copy link
Contributor

PR Analysis

  • 🎯 Main theme: Updating CO2 emission factors for Switzerland
  • 📝 PR summary: This PR updates the CO2 emission factors for Switzerland in the CH.yaml file. The changes include updating the values for different energy sources and adding new entries for the year 2022. The PR also includes formatting changes.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 2, because the PR mainly involves data updates in a YAML file, which are straightforward to review. However, it's important to verify the accuracy of the updated data.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR is well-structured and the changes are clear. However, it would be beneficial to include a brief explanation in the PR description about the source of the new data and why these updates are necessary. This would provide more context to the reviewers and anyone looking at the PR in the future.

  • 🤖 Code feedback:
    relevant fileconfig/zones/CH.yaml
    suggestion      It would be beneficial to add comments in the YAML file next to the updated values, indicating why these specific values were chosen or where they were sourced from. This would provide more transparency and make it easier for others to understand the rationale behind the changes. [medium]
    relevant linevalue: 157.89042611073754

How to use

Instructions

To invoke the PR-Agent, add a comment using one of the following commands:
/review: Request a review of your Pull Request.
/describe: Update the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
/ask <QUESTION>: Ask a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.
/add_docs: Generate docstring for new components introduced in the PR.
/generate_labels: Generate labels for the PR based on the PR's contents.
see the tools guide for more details.

To edit any configuration parameter from the configuration.toml, add --config_path=new_value.
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, add a /config comment.

@@ -27,51 +27,57 @@ emissionFactors:
battery discharge:
- datetime: '2015-01-01'
source: Electricity Maps, 2015 average
value: 127.94998032394102
value: 157.89042611073754
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: The values for the emission factors and the fallback zone mixes have been updated, but the source of these new values is not clear. It would be beneficial to include a comment or a source field indicating where these new values come from.

Suggested change
value: 157.89042611073754
battery discharge:
- datetime: '2015-01-01'
source: Electricity Maps, 2015 average
value: 157.89042611073754
# Updated based on new data from XYZ source

@@ -95,25 +101,28 @@ emissionFactors:
battery discharge:
- datetime: '2015-01-01'
source: Electricity Maps, 2015 average
value: 169.1244092874929
value: 199.24199776578652
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: The values for the emission factors and the fallback zone mixes have been updated, but the source of these new values is not clear. It would be beneficial to include a comment or a source field indicating where these new values come from.

Suggested change
value: 199.24199776578652
battery discharge:
- datetime: '2015-01-01'
source: Electricity Maps, 2015 average
value: 199.24199776578652
# Updated based on new data from XYZ source

@@ -129,25 +138,28 @@ emissionFactors:
hydro discharge:
- datetime: '2015-01-01'
source: Electricity Maps, 2015 average
value: 169.1244092874929
value: 199.24199776578652
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: The values for the emission factors and the fallback zone mixes have been updated, but the source of these new values is not clear. It would be beneficial to include a comment or a source field indicating where these new values come from.

Suggested change
value: 199.24199776578652
hydro discharge:
- datetime: '2015-01-01'
source: Electricity Maps, 2015 average
value: 199.24199776578652
# Updated based on new data from XYZ source

Comment on lines 284 to +287
datetime: '2021-01-01'
value:
battery discharge: 1.5563816477945657e-10
biomass: 0.025025869366431176
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: The values for the emission factors and the fallback zone mixes have been updated, but the source of these new values is not clear. It would be beneficial to include a comment or a source field indicating where these new values come from.

Suggested change
datetime: '2021-01-01'
value:
battery discharge: 1.5563816477945657e-10
biomass: 0.025025869366431176
datetime: '2021-01-01'
value:
battery discharge: 1.5563816477945657e-10
biomass: 0.025025869366431176
coal: 0.053366299671417566
# Updated based on new data from XYZ source

@pierresegonne pierresegonne changed the title Ps/update co2eq params ch Update discharge emissionf factors and overrides CH Dec 18, 2023
@pierresegonne pierresegonne changed the title Update discharge emissionf factors and overrides CH Update discharge emission factors and overrides CH Dec 18, 2023
Copy link
Member

@VIKTORVAV99 VIKTORVAV99 left a comment

Choose a reason for hiding this comment

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

LGTM!

@pierresegonne pierresegonne enabled auto-merge (squash) December 18, 2023 17:45
@pierresegonne pierresegonne merged commit 60da606 into master Dec 18, 2023
11 checks passed
@pierresegonne pierresegonne deleted the ps/update_co2eq_params_CH branch December 18, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
zone config Pull request or issue for zone configurations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants