Uh oh!
There was an error while loading. Please reload this page.
Inflation adjustment usa - #193
Conversation
…o inflation_adjustment_usa
finozzifa
commented
Mar 31, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the handling of inflation rate data by updating the prepare_inflation_rate function and adjusting its usage across tests and cost‐compilation scripts, with a particular focus on handling USD and EUR currency differences. Key changes include:
- Updating test cases to use the new inflation rate file and support both USD and EUR currencies.
- Refactoring the prepare_inflation_rate function and its documentation to accept a currency parameter.
- Removing the inflation adjustment block from pre_process_manual_input_usa and adding inflation adjustment for USD costs in duplicate_fuel_cost.
Reviewed Changes
Copilot reviewed 17 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_helpers.py | Added tests for prepare_inflation_rate with various currency inputs. |
| test/test_compile_cost_assumptions_usa.py | Removed passing of inflation_rate_file_path to pre_process_manual_input_usa. |
| test/test_compile_cost_assumptions.py | Updated inflation_rate file path and removed redundant test for inflation. |
| test/conftest.py | Modified test fixture index for inflation data to include USD and EUR rows. |
| scripts/compile_cost_assumptions_usa.py | Removed inflation adjustment for EUR in pre_process_manual_input_usa and added USD inflation adjustment in duplicate_fuel_cost. |
| scripts/compile_cost_assumptions.py | Removed duplicate definition of prepare_inflation_rate. |
| scripts/_helpers.py | Updated prepare_inflation_rate to support both EUR and USD with conditional row selection. |
Files not reviewed (4)
- Snakefile: Language not supported
- docs/release_notes.rst: Language not supported
- docs/structure.rst: Language not supported
- inputs/US/manual_input_usa.csv: Language not supported
Comments suppressed due to low confidence (3)
scripts/compile_cost_assumptions_usa.py:443
- The removal of the inflation adjustment step in pre_process_manual_input_usa may lead to inconsistent cost estimates for EUR inputs. Please confirm that this change is intentional and update relevant tests and documentation accordingly.
return manual_input_usa_file_df
scripts/_helpers.py:199
- [nitpick] The docstring for prepare_inflation_rate does not mention support for USD inflation rates. Consider updating the documentation to indicate that the function conditionally selects the row based on the 'currency_to_use' parameter.
def prepare_inflation_rate(fn: str, currency_to_use: str = "eur") -> pd.Series:
scripts/compile_cost_assumptions_usa.py:1120
- [nitpick] The variable name 'eur_reference_year' is used when processing USD inflation adjustments. Consider renaming it to more clearly reflect its purpose or usage when applying USD adjustments.
eur_reference_year,
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
finozzifa
commented
Apr 1, 2025
hey @euronion, I think that @danielelerede-oet and myself solved all the comments you posted (thanks again for that <3 ). |
euronion
commented
Apr 2, 2025
Thanks ! This makes reviewing much easier :) Two last (re)quest(ion)s:
|
Hi @euronion thanks for your questions.
|
danielelerede-oet
commented
Apr 2, 2025
@euronion I added a more specific reference linking to the zenodo URL for the model and the specific file we took data from! :D Thank you!!! |
euronion
commented
Apr 2, 2025
Nice, LGTM. RTM? :) |
finozzifa
commented
Apr 2, 2025
Nice. Thanks for your approval |
Closes # (if applicable).
This pull request addresses what detailed in this comment.
The work has been performed by @danielelerede-oet and myself.
Changes proposed in this Pull Request
Checklist
doc.environment.yaml(if applicable).doc/release_notes.rstof the upcoming release is included.