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

Simplify boilerplate #697

Merged
merged 3 commits into from
Mar 9, 2017
Merged

Simplify boilerplate #697

merged 3 commits into from
Mar 9, 2017

Conversation

fpagnoux
Copy link
Member

@fpagnoux fpagnoux commented Mar 6, 2017

Apply openfisca/openfisca-core#467 to france.

CHANGELOG.md Outdated
### 14.1.1 - [#697](https://github.com/openfisca/openfisca-france/pull/697)

* Amélioration technique
* Utilise le module `core.formulas_toolbox` plutôt que de réimporter un par un tous les objets python nécessaire pour écrire une formule
Copy link
Member Author

@fpagnoux fpagnoux Mar 8, 2017

Choose a reason for hiding this comment

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

Rename to model_api

from openfisca_core.formula_helpers import apply_thresholds, switch
from openfisca_core.periods import MONTH, YEAR, ETERNITY

from openfisca_core.formula_toolbox import *
Copy link
Member Author

@fpagnoux fpagnoux Mar 8, 2017

Choose a reason for hiding this comment

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

We can replace the import * by __all__.extend(openfisca_core.module_api.__all__)

Copy link
Member Author

Choose a reason for hiding this comment

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

But at the same time it's much less clear 🤔

from openfisca_core.formula_helpers import apply_thresholds, switch
from openfisca_core.periods import MONTH, YEAR, ETERNITY

from openfisca_core.module_api import *
Copy link
Member

Choose a reason for hiding this comment

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

Did you rename model_api to module_api willingly?

Copy link
Member Author

Choose a reason for hiding this comment

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

No :(
Good catch !

@fpagnoux fpagnoux requested a review from cbenz March 9, 2017 12:22
Copy link
Member

@cbenz cbenz left a comment

Choose a reason for hiding this comment

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

Désolé !

CHANGELOG.md Outdated
### 14.1.1 - [#697](https://github.com/openfisca/openfisca-france/pull/697)

* Amélioration technique
* Utilise le module `core.model_api` plutôt que de réimporter un par un tous les objets python nécessaire pour écrire une formule
Copy link
Member

Choose a reason for hiding this comment

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

Typos :

  • python -> Python
  • nécessaire -> nécessaires

@fpagnoux
Copy link
Member Author

fpagnoux commented Mar 9, 2017

5 fixups pour 2 commits, donc un de changelog. Ne plus ouvrir de PR le soir avant de partir ;)

@fpagnoux fpagnoux requested a review from cbenz March 9, 2017 15:23
@fpagnoux fpagnoux merged commit 27f4b50 into master Mar 9, 2017
@fpagnoux fpagnoux deleted the simplify-boilerplate branch March 9, 2017 16:22
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