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 #798

Merged
merged 22 commits into from
Aug 28, 2017
Merged

Switch parameters to YAML #798

merged 22 commits into from
Aug 28, 2017

Conversation

michelbl
Copy link

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

Connected to openfisca/openfisca-core#523

18.7.7 - #798

  • Amélioration technique
  • Détails :
    • Migration vers la version 17 d'OpenFisca-Core.

@michelbl michelbl changed the title Params yaml Switch parameters to YAML Aug 3, 2017
@michelbl michelbl requested review from fpagnoux and Anna-Livia August 3, 2017 15:14
@michelbl michelbl self-assigned this Aug 3, 2017
Anna-Livia
Anna-Livia previously requested changes Aug 16, 2017
View reviewed changes
CHANGELOG.md Outdated

* Amélioration technique
* Détails :
- Migration vers la version 17 d'OpenFisca-Core.
Copy link
Contributor

Choose a reason for hiding this comment

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

Migration des paramètre de JSON en YAML, connecté à la version 17 d'OpenFisca-Core.

Copy link
Author

Choose a reason for hiding this comment

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

J'ai rajouter une ligne.

Même débat que pour openfisca/extension-template#5

circle.yml Outdated
@@ -9,6 +9,8 @@ dependencies:
override:
- pip install --upgrade pip wheel # pip >= 8.0 needed to be compatible with "manylinux" wheels, used by numpy >= 1.11
- pip install twine
- pip install --editable git+https://github.com/openfisca/openfisca-core.git@params-yaml#egg=OpenFisca-Core
Copy link
Contributor

Choose a reason for hiding this comment

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

Ces éléments sont-ils à retirer lors du merge ? Est ce qu'un commentaire l'indiquant pourrait aider à s'en souvenir ?

Copy link
Author

Choose a reason for hiding this comment

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

Le nom du commit est explicite : "[DO NOT MERGE] CircleCI"

openfisca_france/parameters/_.yaml Outdated
@@ -0,0 +1 @@
type: node
Copy link
Contributor

Choose a reason for hiding this comment

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

Est-ce nécessaire ?

Copy link
Author

Choose a reason for hiding this comment

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

removed

openfisca_france/parameters/bouclier_fiscal/_.yaml Outdated
@@ -0,0 +1,3 @@
description: Bouclier fiscal
Copy link
Contributor

Choose a reason for hiding this comment

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

La valeur ajoutée de ce _.yaml est assez faible

Copy link
Author

Choose a reason for hiding this comment

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

discutté avec Matti

openfisca_france/parameters/bourses_education/_.yaml Outdated
@@ -0,0 +1,3 @@
description: Bourses de l'Éducation nationale
Copy link
Contributor

Choose a reason for hiding this comment

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

cf remarques précédentes

openfisca_france/parameters/bourses_education/bourse_college/_.yaml Outdated
@@ -0,0 +1,3 @@
description: Bourse des collèges
Copy link
Contributor

Choose a reason for hiding this comment

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

cf remarques précédentes

openfisca_france/parameters/bourses_education/bourse_college/apres_2016/_.yaml Outdated
@@ -0,0 +1 @@
type: node
Copy link
Contributor

Choose a reason for hiding this comment

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

cf remarques précédentes

openfisca_france/parameters/bourses_education/bourse_college/apres_2016/echelon_1/_.yaml Outdated
@@ -0,0 +1 @@
type: node
Copy link
Contributor

Choose a reason for hiding this comment

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

cf remarques précédentes

openfisca_france/parameters/bourses_education/bourse_college/apres_2016/echelon_3/_.yaml Outdated
@@ -0,0 +1 @@
type: node
Copy link
Contributor

Choose a reason for hiding this comment

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

cf remarques précédentes

tests/reforms/test_landais_piketty_saez.py Outdated
@@ -1,44 +0,0 @@
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

Pourquoi avoir retiré ces tests ?

Copy link
Author

Choose a reason for hiding this comment

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

C'était temporaire (en attendant de trouver une solution pour les LinearAverateRateTaxScales), j'ai remis ces tests.

@MattiSG MattiSG mentioned this pull request Aug 18, 2017
Closed
@michelbl michelbl requested review from MattiSG and Anna-Livia August 18, 2017 13:28
@MattiSG MattiSG mentioned this pull request Aug 21, 2017
Closed
@MattiSG
Copy link
Member

MattiSG commented Aug 21, 2017

Documenter dans le CONTRIBUTING l'usage des migrations.

@fpagnoux fpagnoux force-pushed the params-yaml branch 3 times, most recently from 3da3610 to 720b5b8 Compare August 28, 2017 20:35
@fpagnoux fpagnoux dismissed Anna-Livia’s stale review August 28, 2017 21:59

Changes have been treated

@fpagnoux fpagnoux merged commit cb27382 into master Aug 28, 2017
@fpagnoux fpagnoux deleted the params-yaml branch August 28, 2017 22:30
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

@MattiSG MattiSG Awaiting requested review from MattiSG

@Anna-Livia Anna-Livia Awaiting requested review from Anna-Livia

Assignees

@michelbl michelbl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@michelbl @MattiSG @fpagnoux @Anna-Livia