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

Currency conversion jrc #434

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Currency conversion jrc #434

merged 3 commits into from
Jan 15, 2025

Conversation

Santonia27
Copy link
Collaborator

@Santonia27 Santonia27 commented Jan 15, 2025

convert jrc damage values from eur 2010 to US dollars 2010

@Santonia27 Santonia27 requested a review from panosatha January 15, 2025 09:16
@Santonia27 Santonia27 changed the title Currency converseion jrc Currency conversion jrc Jan 15, 2025
@Santonia27 Santonia27 requested a review from LuukBlom January 15, 2025 09:21
Copy link
Collaborator

@panosatha panosatha left a comment

Choose a reason for hiding this comment

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

Nice! This will do the job. I think we can merge this but I would make another issue for how to handle the currency in general. I think there should be a currency unit class that includes the currency and the year. Then each dataset will have a currency defined in it (for example JRC will have EUR, 2010 etc.). Then for the conversions maybe we could use something like: https://github.com/MicroPyramid/forex-python. So the user would just provide the unit that they would like their exposure to be in, and then the conversion would be done automatically.

@@ -339,6 +344,7 @@ def setup_buildings_from_multiple_sources(
bf_conversion: bool = False,
keep_unclassified: bool = True,
damage_translation_fn: Union[Path, str] = None,
eur_to_us_dollar: bool = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't the default value be False?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh yes, sorry!

@Santonia27
Copy link
Collaborator Author

Nice! This will do the job. I think we can merge this but I would make another issue for how to handle the currency in general. I think there should be a currency unit class that includes the currency and the year. Then each dataset will have a currency defined in it (for example JRC will have EUR, 2010 etc.). Then for the conversions maybe we could use something like: https://github.com/MicroPyramid/forex-python. So the user would just provide the unit that they would like their exposure to be in, and then the conversion would be done automatically.

Yes I think thats a really nice idea! I hope we can implement this at one point!!

Copy link

@Santonia27 Santonia27 merged commit d28b14e into main Jan 15, 2025
7 checks passed
@Santonia27 Santonia27 deleted the currency_converseion_jrc branch January 15, 2025 09:50
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.

2 participants