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

Add a unit to every legislative parameter #48

Closed
wants to merge 1 commit into from

Conversation

benjello
Copy link
Member

@benjello benjello commented Jul 2, 2018

  • Minor change.
  • Impacted periods: all.
  • Impacted areas: parameters
  • Details:
    • Add a unit to every parameter

@benjello benjello requested a review from fpagnoux July 2, 2018 21:03
@benjello
Copy link
Member Author

benjello commented Jul 2, 2018

@fpagnoux fpagnoux changed the title Add a unit ot every legislative parameter Add a unit to every legislative parameter Jul 5, 2018
@@ -1,4 +1,5 @@
description: Amount of the basic income
unit: currency
Copy link
Member

Choose a reason for hiding this comment

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

Why not EUR or USD instead of currency? These would be proper "units", while currency is more vague

Copy link
Member

Choose a reason for hiding this comment

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

Another option would be to namespace it : currency/EUR.

What do you think @fpagnoux @benjello ?

Copy link
Member

@fpagnoux fpagnoux Jul 6, 2018

Choose a reason for hiding this comment

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

Great minds think alike 😅 openfisca/openfisca-france#1022 (comment)

Copy link
Member

@fpagnoux fpagnoux Jul 6, 2018

Choose a reason for hiding this comment

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

The downside of using / though is that it can be confused with a division. We would have for instance EUR/m2 and currency/EUR where / has a totally different meaning. I guess we could use currency:EUR instead 🤔

@benjello
Copy link
Member Author

benjello commented Jul 6, 2018 via email

@fpagnoux
Copy link
Member

fpagnoux commented Jul 6, 2018

One problem though is that the currency can change over time FRF vs EUR. Ans yes de have all the IR in France since 1917 !

I think @michelbl knew about that when he designed the YAML format, and made sure unit could be specified at the value level.

So you could have something like:

description: montant des AF
unit: currency:EUR
values:
  2015-01-01: 
      value: 200
  ...
  2002-01-01: 
    value: 1000
    unit: currency:FRF
  2001-01-01:
    value: 950
    unit: currency:FRF

@benjello
Copy link
Member Author

benjello commented Jul 6, 2018

@fpagnoux ok then. But it maight be more diffuclt to deal with the code base of France as in here openfisca/openfisca-france#1022 so I suggest that we accept a raw currency too.

@fpagnoux
Copy link
Member

fpagnoux commented Jul 9, 2018

@fpagnoux ok then. But it maight be more diffuclt to deal with the code base of France as in here openfisca/openfisca-france#1022 so I suggest that we accept a raw currency too.

It's a free-text property, so we'll accept everything :).

The discussion seems to have reached consensus, closing in favor of #49

@fpagnoux fpagnoux closed this Jul 9, 2018
@bonjourmauko bonjourmauko deleted the add-parameters-unit branch July 23, 2018 05:55
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.

3 participants