-
Notifications
You must be signed in to change notification settings - Fork 101
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
Revalorisation des allocations familiales au 1er janvier 2019 #1248
Conversation
Merci beaucoup pour ta contribution ! Penses-tu pouvoir faire un refactoring de ces paramètres pour qu'ils correspondent davantage à ce qui es présent dans la législation. Je pense en particulier à
Qu'en penses-tu ? Merci ! |
@guillett effectivement, il faut voir avec @JenniferTelep pour récupérer toutes les valeurs des plafonds de ressources. |
(Rebase et suppression du commit modifiant CHANGELOG/setup - il vaut mieux ne pousser ce commit qu'au dernier moment, quand on est 100% certains de merger la PR dans la foulée.) |
@JenniferTelep, @jmdallais comment voyez vous les choses ? Le refactoring est limité mais permettrait de rapprocher OpenFisca de la législation. |
@frtomas pourrais-tu faire une première review ? |
@guillett Je regarde ça^^ |
Super merci ! |
@frtomas as-tu pu regarder ça ? Merci beaucoup ! |
class af_montant_plafond_tranche_1(Variable): | ||
value_type = float | ||
entity = Famille | ||
label = u"Plafond annuel de ressources n°1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Peut être modifier un peu la description pour plus de clarté, quelque chose comme "Plafond annuel de ressources de la première tranche" ?
class af_montant_plafond_tranche_2(Variable): | ||
value_type = float | ||
entity = Famille | ||
label = u"Plafond annuel de ressources n°2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Même chose, "Plafond annuel de ressources de la deuxième tranche" ?
value_type = float | ||
entity = Famille | ||
label = u"AF - Complément dégressif en cas de dépassement du plafond" | ||
label = u"Depassement mentuel de ressources" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corriger "Depassement mentuel de ressources" en "Depassement mensuel de ressources"
@@ -3,9 +3,13 @@ reference: https://www.ipp.eu/outils/baremes-ipp/ | |||
unit: currency | |||
values: | |||
2015-07-01: | |||
value: 67140.0 | |||
value: 55950.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce qu'il existe une référence pour cette valeur ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non, j'ai pas trouvé la référence pour cette valeur.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -3,9 +3,13 @@ reference: https://www.ipp.eu/outils/baremes-ipp/ | |||
unit: currency | |||
values: | |||
2015-07-01: | |||
value: 89490.0 | |||
value: 78300.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce qu'il existe une référence pour cette valeur ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non, j'ai pas trouvé la référence pour cette valeur.
tests/formulas/af_2019_01.yaml
Outdated
@@ -0,0 +1,110 @@ | |||
- name: Cas N°1 Allocations familiales - Couple, 2 enfants de moins de 14 ans |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il me semble qu'il avait été évoqué de ne pas ajouter de tests au projet pour de simples revalorisations, particulièrement quand elles sont appuyées par des références législatives simples et claires.
Ces tests ont un intérêt pendant la phase de dev pour s'assurer de la justesse et de la fiabilité des calculs mais selon moi ils ne représentent pas assez de valeur ajoutée pour être livrés.
@@ -166,17 +185,9 @@ def formula_2015_07_01(famille, period, parameters): | |||
base_ressources = famille('prestations_familiales_base_ressources', period) | |||
modulation = pfam.modulation | |||
|
|||
plafond1 = ( | |||
modulation.plafond_tranche_1 | |||
+ max_(nb_enf_tot - 2, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quelle est la raison de la disparition de ce max_(nb_enf_tot - 2, 0)
au profit du simple nb_enf_tot
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Les anciennes valeurs des paramètres modulation.plafond_tranche_1 et modulation.plafond_tranche_2 sont calculées pour une famille de 2 enfants et ne sont pas les valeurs de base présentes dans la législation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci pour cette réponse @mtifarine
Effectivement avec cette explication et en regardant à nouveau les anciens commentaires des plafonds tel que 56286 + 5628 * 2
je comprends mieux la modificaiton.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mtifarine @guillett @frtomas Oui c'est vraiment mieux comme ça: d'une part on peut lire les articles de loi pointés par les références législatives, et faire le lien avec les paramètres; d'autre part on n'a plus besoin de recourir à ces commentaires qui auraient empêché d'utiliser le texte des références dans le legislation explorer (qui les traite comme des URL).
Je suis un peu réservé sur l'introduction de nouvelles variables, et je vois encore des possibilités de factorisation, ça vous convient si je reprends la main pour faire une proposition de formulation sans ajout de variables ? Je ne suis pas certain que ça soit possible, ou que ça mène au meilleur résultat, mais je suis curieux de voir ce qu'on peut faire. Si je ne trouve pas mieux on intègrera cette version.
Par ailleurs je vous invite à ne pas modifier vous-même setup.py
et CHANGELOG.md
, je continue à trouver ça plus simple pour la personne qui relit une PR et la mène jusqu'au merge de le faire au tout dernier moment. Autrement, on doit rentrer dans une correction de conflits Git à chaque fois qu'une autre PR a été mergée entre la demande de revue et la revue elle-même, c'est un travail inutile. Je le fais assez souvent pour le ressentir comme une friction :)
Il reste important de bien décrire les modifications apportées dans le commentaire de la PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je profite de cette conversation pour évoquer un sujet qu'on avait abordé à l'oral avec @ThibaultCCMSA lors de notre rencontre à la DSS, mais que je n'avais pas eu l'occasion de traiter entre trève des confiseurs et urgences diverses. (J'associe à la conversation @guillett @frtomas @JenniferTelep - si j'oublie quelqu'un n'hésitez pas à les mentionner dans le fil.)
Je sais que le mode de fonctionnement actuel sur Github avait été adopté d'un commun accord, et il me semble que le début d'année est une bonne occasion pour le revisiter, il y a un point notamment sur lequel je souhaiterais une évolution.
L'idée est la suivante: il est intéressant pour nous d'évaluer l'efficacité du processus de traitement des issues en général sur OpenFisca France en mesurant (entre autres) le temps écoulé entre l'ouverture et la fermeture des issues, ainsi que le "stock" d'issues ouvertes à un moment donné (34 au moment d'écrire ces lignes). Je souhaite maintenir un stock bas et piloter par un temps de traitement le plus court possible.
Avoir une issue globale "Roadmap MSA" qui (par exception) restera ouverte un certain temps est tout à fait compatible avec ce mode de pilotage, et c'est quelque chose qui a bien fonctionné en 2018. Par contre c'est plus pénalisant de maintenir ouvertes de nombreuses issues. Ma préférence serait de ne pas ouvrir d'issue tant que le travail n'est pas démarré; en fait dans ce cas il suffit d'initialiser une PR directement, sans passer par l'étape de création d'une issue.
Seriez-vous OK pour que nous fermions les issues correspondant à la Roadmap MSA #1130, en prévoyant de les ré-ouvrir au fur et à mesure que le travail sur chacune est commencé (et les fermer à nouveau lorsqu'il est terminé) ?
Si vous préférez, on peut garder le stock actuel sans les fermer, et je serais alors demandeur qu'on adopte ce nouveau mode de fonctionnement pour les travaux futurs: une seule issue "roadmap" pour consolider le calendrier des travaux à venir, déclinées seulement en PR lorsque le travail commence.
Je suis à l'écoute de vos remarques bien entendu :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Morendil si tu as une idée de factorisation du code ne nécessitant pas d'ajout de variables, on te laisse volontiers reprendre la main et vérifier la faisabilité.
Concernant le second point, il faut effectivement voir ça avec @JenniferTelep et @ThibaultCCMSA.
+ max_(nb_enf_tot - 2, 0) | ||
* modulation.majoration_plafond_par_enfant_supplementaire | ||
) | ||
plafond1 = modulation.plafond_tranche_1 + nb_enf_tot * modulation.majoration_plafond_par_enfant_supplementaire |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi ne pas réemployer ces appels : plafond1 = famille('af_montant_plafond_tranche_1', period)
?
A moins que je rate quelque chose à la lecture, il me semble qu'il s'agit exactement de la même formule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pas le même nombre d'enfants : nb_enf_tot = af_nbenf + af_forfaitaire_nbenf
Désolé d'arriver après la bataille mais je pense qu'il faut faire attention quand on modifie les défnitions de certains paramètres. Je pense au plafond qui était donnée précédemment par le plafond pour 2 enfants (car il n'est pas possible d'avoir des AF pour 0 ou 1 enfant donc la valeur retenue était celle qui avait le plus de sens) et qui a été remplacée par la valeur "hors enfants". Sans arbitrer sur le fond, je pense qu'il vaut mieux créer des paramètres sous un nouveau nom plutôt que modifier des anciens paramètres notamment quad la référence est bareme-ipp. En tout cas cela me semble préférable avant le grand merge à venir. |
@benjello je suis d'accord avec toi et je pense que cette PR devra faire l'objet d'un changement de version majeure. My 2cts. |
@benjello @guillett Telles que j'avais compris les choses, nous essayons de nous rapprocher au mieux des textes.
C’est la vision que j'en avais, mais on se pliera au choix du plus grand nombre ^^ |
@mtifarine : je suis d'accord avec toi sur le principe qu'il faille coller aux textes. Pour être plus explicite, ma remarque s'appuie sur deux éléments:
Je préfère donc que l'on préserve les noms actuels, et le fonctionnement actuel sans surprise et que m'on adopte un objectif de meilleure qualité des paramètres ex-post. |
Pour clarification, je soutiens le refactoring qui a été fait (car il a beaucoup de valeur étant donné qu'il permet de coller aux textes à la lettre) et la montée en version (pour le principe de moindre surprise). |
Pour clarification sur la clarification de @guillett : ok aussi mais attention sur le nom des paramètres. |
L'intégration continue/CircleCI a évolué pour builder sur python 3 uniquement. |
@mtifarine Je pense qu'on a eu un souci de coordination sur cette PR, j'avais force-pushé des commits à un moment et tu as poussé Comme on en avait déjà discuté je vais pousser une nouvelle version avec une autre façon de factoriser les parties de code en commun, qui sera un helper; ça n'est pas une solution satisfaisante sur le long terme et c'est sans doute la meilleure tant qu'on a pas lancé des travaux sur Core pour permettre de traiter plus facilement la situation où on est ici, c'est-à-dire de pouvoir appeler depuis une formule une autre formule avec des valeurs différentes pour la quantité "nombre d'enfants" ( Je garde donc la main sur cette PR et elle devrait aboutir dans la semaine. |
@benjello Bien vu, on a effectivement changé le sens du paramètre en lui gardant le même nom. On pourrait effectivement garder en parallèle les anciens paramètres et les nouveaux. Je vais étudier l'option. |
082d8ec
to
f910f9d
Compare
@mtifarine @guillett @benjello @frtomas J'ai l'intention de merger en fin de journée cette dernière version qui complète le travail de @mtifarine avec:
J'attire votre attention sur la line 260 de cette version modifée, on calcule |
prestations/prestations_familiales/af/modulation
.