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.

Sign up for GitHub

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

Jump to bottom

Switch parameters to YAML #17

Merged
merged 13 commits into from
Aug 28, 2017
Merged

Switch parameters to YAML #17

merged 13 commits into from
Aug 28, 2017

Conversation

michelbl
Copy link
Contributor

@michelbl michelbl commented Aug 3, 2017 •
edited by MattiSG
Loading

Connected to openfisca/openfisca-core#523

1.2.4 - #17

  • Technical improvement: adapt to version 17.0.0 of Openfisca-Core
  • Details:
    • Transform XML parameter files to YAML parameter files.

@michelbl michelbl force-pushed the params-yaml branch 2 times, most recently from f099cfb to f45f267 Compare August 3, 2017 13:21
@michelbl michelbl requested review from fpagnoux and Anna-Livia August 3, 2017 15:14
@michelbl michelbl self-assigned this Aug 3, 2017
fpagnoux
fpagnoux requested changes Aug 4, 2017
View reviewed changes
Copy link
Member

@fpagnoux fpagnoux left a comment

Choose a reason for hiding this comment

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

That's some big improvement 🎉 !

Note sure about this type attribute though.

openfisca_country_template/parameters/_.yaml Outdated
@@ -0,0 +1,2 @@
# see https://doc.openfisca.fr/parameters.html
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need this almost empty file ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discussed with Anna-Livia and Matti

openfisca_country_template/parameters/benefits/_.yaml Outdated
@@ -0,0 +1 @@
type: node
Copy link
Member

Choose a reason for hiding this comment

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

Same question here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

openfisca_country_template/parameters/taxes/income_tax_rate.yaml Outdated
description: Income tax rate
type: parameter
values:
'2012-01-01':
Copy link
Member

Choose a reason for hiding this comment

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

Are these quotes necessary ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

openfisca_country_template/reforms/modify_social_security_taxation.py Outdated

# Access the right legislation node:
scale = reference_legislation_json_copy['children']['taxes']['children']['social_security_contribution']
brackets = scale['brackets']
brackets = legislation.taxes.social_security_contribution.brackets
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄

@michelbl michelbl requested review from MattiSG and fpagnoux August 18, 2017 12:49
@fpagnoux fpagnoux merged commit d79de54 into master Aug 28, 2017
@fpagnoux fpagnoux deleted the params-yaml branch August 28, 2017 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpagnoux fpagnoux fpagnoux approved these changes

Assignees

@michelbl michelbl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@michelbl @fpagnoux