-
Notifications
You must be signed in to change notification settings - Fork 217
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
Universal currency conversion #1319
base: main
Are you sure you want to change the base?
Universal currency conversion #1319
Conversation
…a-earth into add_costs_usa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @danielelerede-oet, thank you for the great contribution!
Looks really nice, and will be definitely very helpful. Have added a couple of quick comments which mainly relate to the maintenance part. Could you please have a look?
doc/release_notes.rst
Outdated
* Introduce universal currency conversion | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be great to adjust the note formatting to match the format of the others for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done! :)
@@ -89,3 +89,4 @@ dependencies: | |||
- git+https://github.com/davide-f/google-drive-downloader@master # google drive with fix for virus scan | |||
- chaospy # lastest version only available on pip | |||
- fake_useragent | |||
- currencyconverter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have there been any particular reasons to use pip
distribution? As discussed, pip
can easily lead to the dependencies conflicts and must be avoided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow-up of our discussion, a requirement to have conda
installation is by no means ultimative and can be also managed on the maintenance side if there is no other way around 🙂
envs/linux-pinned.yaml
Outdated
@@ -575,6 +575,7 @@ dependencies: | |||
- zstd=1.5.6 | |||
- pip: | |||
- chaospy==4.3.17 | |||
- currencyconverter==0.17.30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note please that pinned environments are being automatically updated (e.g. #1321 is an example of an automated PR which fixes this issue). So, luckily there is no need for manual changes
envs/macos-pinned.yaml
Outdated
@@ -503,6 +503,7 @@ dependencies: | |||
- zstd=1.5.6 | |||
- pip: | |||
- chaospy==4.3.17 | |||
- currencyconverter==0.17.30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above: no need for manual changes
envs/windows-pinned.yaml
Outdated
@@ -502,6 +502,7 @@ dependencies: | |||
- zstd=1.5.6 | |||
- pip: | |||
- chaospy==4.3.17 | |||
- currencyconverter==0.17.30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above: no need for manual changes
… into add_costs_usa
The changes in data/costs.csv are needed to align results of the tests with the previous version. I added the "further description" header to uniform with the standard structure of cost files from |
Changes proposed in this Pull Request
Hi, @finozzifa and I have been working on universal currency conversion.
In this PR, universal currency conversion is introduced taking advantage of the Currency_Converter package.
Currently, the only allowed conversion adopts a fixed USD to EUR exchange rate for the year 2013. Here, any currency among those in the list at https://github.com/alexprengere/currencyconverter/blob/master/currency_converter/eurofxref.csv may be introduced in the PyPSA-Earth input dataset and converted into the
output_currency
specified in the config file undercosts
according to the specifiedcurrency_year
for each investment, FOM and VOM cost (the adopted conversion factor will correspond to the yearly average for the specifiedcurrency_year
, according to theget_yearly_currency_exchange_average
in_helpers.py
).Additionally, in this way results can be obtained in any currency the user prefers (according to the specification of
output_currency
) - EUR is set as default as it is in the current PyPSA-Earth.The only caveat at the moment is that cost units must be specified via code (i.e., USD, EUR, etc. instead of $, €). That is not an issue as the current
technology_data
output files only contain codes, but a PR will be also raised totechnology_data
to address the conversion from symbol to code, if necessary (something similar is already there now, but only acts on some input data).Checklist
envs/environment.yaml
anddoc/requirements.txt
.config.default.yaml
andconfig.tutorial.yaml
.test/
(note tests are changing the config.tutorial.yaml)doc/configtables/*.csv
and line references are adjusted indoc/configuration.rst
anddoc/tutorial.rst
.doc/release_notes.rst
is amended in the format of previous release notes, including reference to the requested PR.