Spécifier toujours une période dans les appels de variables #699

michelbl · 2017-03-08T13:57:27Z

Connected to openfisca/openfisca-core#468

Amélioration technique
Périodes concernées : toutes
Zones impactées : revenus/activite/salarie/salaire_net_a_payer, réformes et tests
Détails :
- Spécifie toujours une période dans les appels de variables, dans les formules et dans les tests.

Ces changements :

Adaptent france à la version 7.0.0 de core.
Corrigent des calculs déjà existants.

michelbl · 2017-03-08T14:06:03Z

Related to openfisca/openfisca-core#469

fpagnoux

Merci pour la rapidité !

Le code est ok pour moi, juste quelques remarques sur le changelog.

fpagnoux · 2017-03-08T16:33:31Z

requirements.txt

@@ -1,5 +1,5 @@
 # Add Biryani extra requirement here because it is needed by OpenFisca-Core and installation from Git URL below
 # does not take extras_require of Core setup.py into account.
 Biryani[datetimeconv] >= 0.10.4
--editable git+https://github.com/openfisca/openfisca-core.git#egg=OpenFisca-Core
+--editable git+https://github.com/openfisca/openfisca-core.git@mandatory-period#egg=OpenFisca-Core


@cbenz il me semblait qu'on avait un script qui gérait ça automatiquement, il a été supprimé ? Ou il ne fonctionne juste plus ?

Il est toujours là, mais on avait décidé (à l'oral) de supprimer cela sachant que ça introduisait un comportement automatique et qu'on pouvait changer la branche dans requirements.txt.

Coïncidence : je viens de poster une PR sur Web-API qui supprime le script en question sur le dépôt Web-API.

fpagnoux · 2017-03-08T16:34:52Z

openfisca_france/model/revenus/activite/salarie.py

@@ -891,7 +891,7 @@ def function(self, simulation, period):
        '''
        salaire_net = simulation.calculate_add('salaire_net', period)
        depense_cantine_titre_restaurant_employe = simulation.calculate(
-            'depense_cantine_titre_restaurant_employe')
+            'depense_cantine_titre_restaurant_employe', period)


C'était probablement une erreur je pense. @laem tu confirmes ?

@fpagnoux je n'utilise pas cette variable, c'est hors-champ dans le simulateur pour l'instant (c'est le monde des conventions collectives et accords entreprise), mais j'imagine que oui c'était un oubli.

fpagnoux · 2017-03-08T16:38:54Z

CHANGELOG.md

+## 14.2.0 - [#685](https://github.com/openfisca/openfisca-france/pull/699)
+
+* Amélioration technique
+* Périodes concernées : toutes


Cette ligne et la suivante ne sont nécessaires que dans le cas d'une évolution de la legislation. On peut les enlever ici.

fpagnoux · 2017-03-08T16:40:14Z

CHANGELOG.md

+* Détails :
+  - Spécifie toujours une période dans les appels de variables, dans les formules et dans les tests.
+
+Ces changements :


Ce second bloc devrait être intégrés dans le bloc standardisé.

fpagnoux · 2017-03-08T16:42:32Z

CHANGELOG.md

+Ces changements :
+  - Adaptent `france` à  la version `7.0.0` de `core`.
+  - Corrigent des calculs déjà existants.
+


En fait cette PR fait deux choses :

Elle adapte le package france au nouveau core, ce qui est une Amélioration technique

Elle corrige une erreur dans le calcul de salaire_net_a_payer, ce qui est une Évolution du système socio-fiscal.

Je pense que ce serait plus lisible d'avoir deux blocs séparés.

michelbl · 2017-03-09T09:07:32Z

@fpagnoux j'ai essayé de prendre en compte tes remarques sur le Changelog. Si tu estimes que c'est encore améliorable, n'hésite pas à éditer toi-même la branche (ou me dire quoi changer)

fpagnoux · 2017-03-09T12:36:19Z

Done.

Pour les Évolution du système socio-fiscal, l'idée est de se mettre du point de vue du réutilisateur, et de vraiment expliciter les impacts sur le calcul, sans se préoccuper des aspects techniques (tels que "Spécifie toujours une période dans les appels de variables, dans les formules et dans les tests.")

fpagnoux · 2017-03-09T15:16:35Z

setup.py

@@ -45,7 +45,7 @@
        'Babel >= 0.9.4',
        'Biryani[datetimeconv] >= 0.10.4',
        'numpy >= 1.11',
-        'OpenFisca-Core >= 6.0.0, < 7.0',
+        'OpenFisca-Core >= 6.0.0, < 8.0',


@michelbl On avait oublié ça

The `period` argument is no longer optional.

A call was made without a period. The period of the simulation was silently used instead. This introduced a computation error in cases where the period of the simulation was different.

Some variables requested computations without providing a period. The period of the simulation was used instead, potentially introducing computation errors.

michelbl requested review from cbenz and fpagnoux March 8, 2017 13:58

michelbl self-assigned this Mar 8, 2017

michelbl force-pushed the mandatory-period branch from 899d610 to e5a6f60 Compare March 8, 2017 14:02

fpagnoux requested changes Mar 8, 2017

View reviewed changes

michelbl force-pushed the mandatory-period branch from e5a6f60 to ed82b7e Compare March 9, 2017 09:05

michelbl requested a review from fpagnoux March 9, 2017 09:08

fpagnoux approved these changes Mar 9, 2017

View reviewed changes

cbenz approved these changes Mar 9, 2017

View reviewed changes

fpagnoux reviewed Mar 9, 2017

View reviewed changes

Michel Blancard and others added 6 commits March 9, 2017 17:23

Provide a period for computations in tests

f594b92

The `period` argument is no longer optional.

Fix variable salaire_net_a_payer

15ff742

A call was made without a period. The period of the simulation was silently used instead. This introduced a computation error in cases where the period of the simulation was different.

Fix reforms

c08812b

Some variables requested computations without providing a period. The period of the simulation was used instead, potentially introducing computation errors.

Mettre à jour le Changelog

ebdbedd

Mettre à jour le numéro de version

61e72c8

Declare this module compatible with core 7

4a05478

fpagnoux force-pushed the mandatory-period branch from 05fa4aa to 4a05478 Compare March 9, 2017 16:26

fpagnoux merged commit 0eafb26 into master Mar 9, 2017

fpagnoux deleted the mandatory-period branch March 9, 2017 16:45

MattiSG mentioned this pull request Apr 11, 2017

Test coherence of cotisations sociales between montlhy and yearly cotisations #611

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spécifier toujours une période dans les appels de variables #699

Spécifier toujours une période dans les appels de variables #699

michelbl commented Mar 8, 2017 •

edited by fpagnoux

Loading

michelbl commented Mar 8, 2017

fpagnoux left a comment

fpagnoux Mar 8, 2017

cbenz Mar 9, 2017

fpagnoux Mar 8, 2017

laem Mar 9, 2017 •

edited

Loading

fpagnoux Mar 8, 2017

michelbl Mar 9, 2017

fpagnoux Mar 8, 2017

michelbl Mar 9, 2017

fpagnoux Mar 8, 2017

michelbl commented Mar 9, 2017

fpagnoux commented Mar 9, 2017 •

edited

Loading

fpagnoux Mar 9, 2017

michelbl Mar 9, 2017

Spécifier toujours une période dans les appels de variables #699

Spécifier toujours une période dans les appels de variables #699

Conversation

michelbl commented Mar 8, 2017 • edited by fpagnoux Loading

michelbl commented Mar 8, 2017

fpagnoux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laem Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michelbl commented Mar 9, 2017

fpagnoux commented Mar 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michelbl commented Mar 8, 2017 •

edited by fpagnoux

Loading

laem Mar 9, 2017 •

edited

Loading

fpagnoux commented Mar 9, 2017 •

edited

Loading