Ajoute une première version du dispositif départ 18-25 #1507

guillett · 2021-03-30T22:04:41Z

Évolution du système socio-fiscal.
Périodes concernées : à partir du 05/09/2014.
Zones impactées : prestations/depart1825.py.
Détails :
- Ajoute une première version du dispositif départ 18-25

Ces changements (effacez les lignes ne correspondant pas à votre cas) :

Ajoutent une fonctionnalité (par exemple ajout d'une variable).

Quelques conseils à prendre en compte :

Jetez un coup d'œil au guide de contribution.
Regardez s'il n'y a pas une proposition introduisant ces mêmes changements.
Documentez votre contribution avec des références législatives.
Mettez à jour ou ajoutez des tests correspondant à votre contribution.
Augmentez le numéro de version dans setup.py.
Mettez à jour le CHANGELOG.md.
Assurez-vous de bien décrire votre contribution, comme indiqué ci-dessus

Et surtout, n'hésitez pas à demander de l'aide ! :)

MattiSG

Je suis très gêné par le nombre de variables introduites. J'y vois une approche très pertinente d'un point de vue de génie logiciel, avec de l'encapsulation de fonctions, mais qui me paraît inappropriée pour la modélisation de la loi en code : chacun des éléments apportés n'a pas de sens d'un point de vue législatif, et le nombre de variables introduites dans l'espace de nommage global rend leur utilisation très compliquée.

openfisca_france/model/prestations/depart1825.py

guillett · 2021-04-07T06:36:22Z

Après plusieurs années d'écriture, d'investigation et de revue de code OpenFisca, je ne sais plus écrire des variables qui contiennent beaucoup de complexité et de logique. Je peux comprendre la gêne ressentie par la création de nombreuses variables mais, selon moi, elles ont bien chacune un seus d'un point de vue législatif (pris au sens très large, étant donné qu'ici, la loi, c'est de la documentation ANCV).

La création de plusieurs variables permet aussi de créer des espaces de complexité séparés où les liens avec le restes de la modélisation du système socio-fiscal sont explicités et sont disposés à une sorte de frontière avec un nouvel espace de complexité. Cela facilite grandement la rédaction de tests plus unitaires et la réutilisation du moteur (lorsque l'on souhaite calculer qu'une variable en particulier).

openfisca_france/parameters/prestations/depart1825.yml

MattiSG · 2021-04-07T09:53:48Z

openfisca_france/parameters/prestations/depart1825.yml

+ 2014-09-05:
+ value: 17000


Quand on ne renseigne que value, je trouve plus lisible d'utiliser la notation condensée 2014-09-05: 17000 🙂

openfisca_france/parameters/prestations/depart1825.yml

openfisca_france/model/prestations/depart1825.py

Plus de point bloquant.

MattiSG

Super ! Ce dispositif me semble pertinent à ajouter tel quel en l'absence de lignes directrices sur l'éclatement des variables intermédiaires.

Je me permets de suggérer d'attendre jusqu'à demain fin d'après-midi avant d'intégrer puisque nous avons une discussion prévue précisément sur ce sujet et que cette implémentation me semble être un bon cas d'exemple, qui pourrait donc se retrouver impacté en fonction des résultats, et dont tout changement nécessitera un changement de version majeur 🙂

openfisca_france/parameters/prestations/depart1825.yml

guillett · 2021-06-18T09:37:23Z

@MattiSG tu avais déjà validé la PR mais avec des réserves, donc je me suis permis de t'en redemander une.

J'ai supprimé une variable (le calcul du plafond de ressources) mais conservé les variables d'éligibilité. Est-ce Ok pour toi ? Merci.

MattiSG

Est-ce Ok pour toi ?

Je persiste à penser que l'éclatement de depart1825_eligibilite en depart1825_eligibilite_statut et depart1825_eligibilite_financiere n'apporte que très peu de lisibilité supplémentaire par l'éclatement en deux formules (lisibilité que l'on pourrait tout aussi bien retrouver avec deux blocs de code introduits par un commentaire ou en nommant une variable interne à la fonction), voire même qu'au contraire le nombre de lignes ajoutées est plutôt contre-productif pour la compréhensibilité au vu de la quantité de métadonnées nécessaires pour OpenFisca ; et que dans tous les cas ces variables supplémentaires exposées publiquement n'ont pas de valeur de réutilisabilité et au contraire la diminuent en demandant une exploration et un choix supplémentaires pour les réutiliser.

Je joins à cette revue une proposition de refactor qui illustre pour moi la lisibilité d'une seule formule, avec 30% de code en moins, et une seule variable publiquement exposée.

MattiSG · 2021-06-20T12:45:23Z

openfisca_france/model/prestations/depart1825.py

+class depart1825_eligibilite_financiere(Variable):
+ value_type = bool
+ label = "Éligibilité au dispositif Départ 18-25 en raison des ressources du foyer"
+ entity = Individu
+ definition_period = MONTH
+ reference = [
+ "https://programme-depart-1825.com/eligibilite/",
+ "https://www.ancv.com/actualites/le-magazine/depart-1825-un-nouveau-programme-pour-les-jeunes-de-18-25-ans"
+ ]
+
+ def formula(individu, period, parameters):
+ nbptr = individu.foyer_fiscal('nbptr', period.n_2)
+ plafond_ressources = parameters(period).prestations.depart1825.plafond_ressources
+ plafond_ressources = plafond_ressources.base + 2 * max_(0, nbptr - 1) * plafond_ressources.par_demi_part_supplementaire
+
+ ressources = individu.foyer_fiscal('rfr', period.n_2)
+ return ressources <= plafond_ressources
+
+
+class depart1825_eligibilite_statut(Variable):
+ """
+ Situations non prises en compte
+ - en contrat aidé,
+ - inscrit dans une école de la deuxième chance,
+ - volontaire en service civique,
+ - jeunes suivis par l'Aide Sociale à l'Enfance.
+ """
+ value_type = bool
+ label = "Éligibilité au dispositif Départ 18-25 en raison de la situation du jeune"
+ entity = Individu
+ definition_period = MONTH
+ reference = [
+ "https://programme-depart-1825.com/eligibilite/"
+ ]
+
+ def formula(individu, period):
+ etudiant_boursier = (individu('activite', period) == TypesActivite.etudiant) * individu('boursier', period)
+ alternant = individu('alternant', period)
+ garantie_jeunes = individu('garantie_jeunes', period) > 0
+
+ return etudiant_boursier + alternant + garantie_jeunes
+
+
+class depart1825_eligibilite(Variable):
+ value_type = bool
+ label = "Éligibilité au dispositif départ 18-25"
+ entity = Individu
+ definition_period = MONTH
+ reference = [
+ "https://programme-depart-1825.com/eligibilite/",
+ "https://www.ancv.com/actualites/le-magazine/depart-1825-un-nouveau-programme-pour-les-jeunes-de-18-25-ans"
+ ]
+
+ def formula(individu, period, parameters):
+ criteres_age = parameters(period).prestations.depart1825.age
+ age = individu('age', period)
+
+ eligibilite_statut = individu('depart1825_eligibilite_statut', period)
+ eligibilite_financiere = individu('depart1825_eligibilite_financiere', period)
+ eligible = eligibilite_statut + eligibilite_financiere
+
+ return (criteres_age.minimum <= age) * (age <= criteres_age.maximum) * eligible


Suggested change

class depart1825_eligibilite_financiere(Variable):

value_type = bool

label = "Éligibilité au dispositif Départ 18-25 en raison des ressources du foyer"

entity = Individu

definition_period = MONTH

reference = [

"https://programme-depart-1825.com/eligibilite/",

"https://www.ancv.com/actualites/le-magazine/depart-1825-un-nouveau-programme-pour-les-jeunes-de-18-25-ans"

]

def formula(individu, period, parameters):

nbptr = individu.foyer_fiscal('nbptr', period.n_2)

plafond_ressources = parameters(period).prestations.depart1825.plafond_ressources

plafond_ressources = plafond_ressources.base + 2 * max_(0, nbptr - 1) * plafond_ressources.par_demi_part_supplementaire

ressources = individu.foyer_fiscal('rfr', period.n_2)

return ressources <= plafond_ressources

class depart1825_eligibilite_statut(Variable):

"""

Situations non prises en compte

- en contrat aidé,

- inscrit dans une école de la deuxième chance,

- volontaire en service civique,

- jeunes suivis par l'Aide Sociale à l'Enfance.

"""

value_type = bool

label = "Éligibilité au dispositif Départ 18-25 en raison de la situation du jeune"

entity = Individu

definition_period = MONTH

reference = [

"https://programme-depart-1825.com/eligibilite/"

]

def formula(individu, period):

etudiant_boursier = (individu('activite', period) == TypesActivite.etudiant) * individu('boursier', period)

alternant = individu('alternant', period)

garantie_jeunes = individu('garantie_jeunes', period) > 0

return etudiant_boursier + alternant + garantie_jeunes

class depart1825_eligibilite(Variable):

value_type = bool

label = "Éligibilité au dispositif départ 18-25"

entity = Individu

definition_period = MONTH

reference = [

"https://programme-depart-1825.com/eligibilite/",

"https://www.ancv.com/actualites/le-magazine/depart-1825-un-nouveau-programme-pour-les-jeunes-de-18-25-ans"

]

def formula(individu, period, parameters):

criteres_age = parameters(period).prestations.depart1825.age

age = individu('age', period)

eligibilite_statut = individu('depart1825_eligibilite_statut', period)

eligibilite_financiere = individu('depart1825_eligibilite_financiere', period)

eligible = eligibilite_statut + eligibilite_financiere

return (criteres_age.minimum <= age) * (age <= criteres_age.maximum) * eligible

class depart1825_eligibilite(Variable):

"""

Situations non prises en compte

- en contrat aidé,

- inscrit dans une école de la deuxième chance,

- volontaire en service civique,

- jeunes suivis par l'Aide Sociale à l'Enfance.

"""

value_type = bool

label = "Éligibilité au dispositif départ 18-25"

entity = Individu

definition_period = MONTH

reference = [

"https://programme-depart-1825.com/eligibilite/",

"https://www.ancv.com/actualites/le-magazine/depart-1825-un-nouveau-programme-pour-les-jeunes-de-18-25-ans"

]

def formula(individu, period, parameters):

criteres_age = parameters(period).prestations.depart1825.age

age = individu('age', period)

eligibilite_age = (criteres_age.minimum <= age) * (age <= criteres_age.maximum)

etudiant_boursier = (individu('activite', period) == TypesActivite.etudiant) * individu('boursier', period)

alternant = individu('alternant', period)

garantie_jeunes = individu('garantie_jeunes', period) > 0

eligibilite_statut = etudiant_boursier + alternant + garantie_jeunes

nombre_parts = individu.foyer_fiscal('nbptr', period.n_2)

plafond_ressources = parameters(period).prestations.depart1825.plafond_ressources

plafond_ressources = plafond_ressources.base + 2 * max_(0, nbptr - 1) * plafond_ressources.par_demi_part_supplementaire

ressources = individu.foyer_fiscal('rfr', period.n_2)

eligibilite_financiere = ressources <= plafond_ressources

return eligibilite_age * (eligibilite_statut + eligibilite_financiere)

MattiSG · 2021-07-08T08:28:47Z

Hello @QuentinMadura !

Je vois qu'il y a de nombreux pushes et force-pushes sur cette PR ouverte il y a trois mois et déjà revue à deux reprises. Or :

Chaque modification publique de ce type génère une notification par mail, ce qui fait pas mal de bruit.
Il devient impossible de savoir quand une PR est réellement prête pour être revue ou non. Cette insécurité réduit la probabilité d'une revue, et en augmente le délai.

En l'occurrence, cette PR disposait déjà d'une revue acceptée. Je ne sais plus si cette acceptation est encore valide à ce stade au vu du nombre de commits ultérieurs, ce qui me pose d'autant plus problème que c'est moi qui avais accepté : je ne sais pas si je peux encore prendre à ce stade la responsabilité d'une validation donnée il y a plusieurs semaines !

Mes demandes sont donc de :

Minimiser le nombre de pushes une fois la PR ouverte (quitte à la rebasculer en draft ou la fermer si des modifications importantes sont prévues, en expliquant sur quel sujet).
N'utiliser le force-push qu'après une revue, pour un rebase, et sans modifier d'aucune façon le code final.
Indiquer dans le fil de conversation si des modifications importantes sont prévues, et sur quel thème, ultérieurement à l'ouverture de la PR.

Est-ce que cela te semble assez clair ? Est-ce acceptable ? 🙂

Bonne journée,
Matti

QuentinMadura · 2021-07-09T10:08:54Z

Hello @QuentinMadura !

Je vois qu'il y a de nombreux pushes et force-pushes sur cette PR ouverte il y a trois mois et déjà revue à deux reprises. Or :

Chaque modification publique de ce type génère une notification par mail, ce qui fait pas mal de bruit.

Il devient impossible de savoir quand une PR est réellement prête pour être revue ou non. Cette insécurité réduit la probabilité d'une revue, et en augmente le délai.

En l'occurrence, cette PR disposait déjà d'une revue acceptée. Je ne sais plus si cette acceptation est encore valide à ce stade au vu du nombre de commits ultérieurs, ce qui me pose d'autant plus problème que c'est moi qui avais accepté : je ne sais pas si je peux encore prendre à ce stade la responsabilité d'une validation donnée il y a plusieurs semaines !

Mes demandes sont donc de :

Minimiser le nombre de pushes une fois la PR ouverte (quitte à la rebasculer en draft ou la fermer si des modifications importantes sont prévues, en expliquant sur quel sujet).

N'utiliser le force-push qu'après une revue, pour un rebase, et sans modifier d'aucune façon le code final.

Indiquer dans le fil de conversation si des modifications importantes sont prévues, et sur quel thème, ultérieurement à l'ouverture de la PR.

Est-ce que cela te semble assez clair ? Est-ce acceptable ? 🙂

Bonne journée,
Matti

Bonjour @MattiSG,

merci de ton retour et pour tes commentaires, et cela me semble clair et acceptable :) !

Effectivement je me rends compte que d'amender un commit n'est pas une bonne pratique et rajoute du bruit pour les autres contributeurs (notification mail, etc.) en plus de l'insécurité, augmentation du délai..

Je tâcherai de faire attention à ces remarques lors de mes prochaines contributions.

Merci d'avoir pris le temps et bonne journée,
Quentin

guillett · 2021-07-29T09:54:49Z

Je considère mergeable. Je vais faire un rebase et les modifications du Changelog et de setup avec pour objectif de merge dans l'après midi.

Co-authored-by: Matti Schneider <matti@mes-aides.org>

guillett added the aides-jeunes label Mar 30, 2021

guillett force-pushed the depart1825 branch from 5bfa7cf to acb1deb Compare April 1, 2021 12:40

MattiSG previously requested changes Apr 6, 2021

View reviewed changes

openfisca_france/model/prestations/depart1825.py Outdated Show resolved Hide resolved

openfisca_france/model/prestations/depart1825.py Outdated Show resolved Hide resolved

guillett requested a review from MattiSG April 7, 2021 06:36

MattiSG reviewed Apr 7, 2021

View reviewed changes

guillett mentioned this pull request Apr 7, 2021

Ajoute le calcul de l'éligibilité et du montant de la caution Visale pour les moins de 30 ans #1513

Merged

4 tasks

guillett force-pushed the depart1825 branch from 6a0f841 to e0f3d4d Compare April 9, 2021 15:44

guillett requested a review from MattiSG April 13, 2021 09:33

MattiSG approved these changes Apr 26, 2021

View reviewed changes

openfisca_france/parameters/prestations/depart1825.yml Outdated Show resolved Hide resolved

openfisca_france/parameters/prestations/depart1825.yml Outdated Show resolved Hide resolved

openfisca_france/parameters/prestations/depart1825.yml Outdated Show resolved Hide resolved

MattiSG force-pushed the depart1825 branch from 62e4740 to afc7324 Compare May 12, 2021 16:43

guillett requested a review from MattiSG June 18, 2021 09:36

MattiSG reviewed Jun 20, 2021

View reviewed changes

QuentinMadura force-pushed the depart1825 branch 4 times, most recently from 10ac672 to b91f3b1 Compare July 8, 2021 07:52

Thomas Guillet and others added 8 commits July 29, 2021 11:55

Ajoute une première version du dispositif départ 18-25

80af3bb

Ajoute une référence au plafond de ressources départ 18-25

38f353d

Ajoute une référence à l'éligibilite financière de départ 18-25

a2aa13d

Apply suggestions from code review

5629304

Co-authored-by: Matti Schneider <matti@mes-aides.org>

Adapte les tests et paramètres suite la revue

c3b0e57

Ajoute une référence

50c8cce

Add units and documentation to parameters

f3c8eed

Use longhand syntax in parameters

e815107

guillett and others added 7 commits July 29, 2021 11:55

Supprime une variable intermédiaire

2ba3d07

Adapte la formule d'éligibilité suite à la revue de code

df79d20

Mets à jour le départ 18-25 ans

9d61ae7

Mets à jour les tests suite à la revue de code

3d5ded4

Aère le fichier de départ 18-25

e41cb95

Lint les fichiers

434a85d

Met à jour CHANGELOG et setup.py

b7c4ee8

guillett force-pushed the depart1825 branch from 550c1ef to b7c4ee8 Compare July 29, 2021 09:59

guillett merged commit 74ad1b8 into master Jul 29, 2021

guillett deleted the depart1825 branch July 29, 2021 13:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ajoute une première version du dispositif départ 18-25 #1507

Ajoute une première version du dispositif départ 18-25 #1507

guillett commented Mar 30, 2021

MattiSG left a comment

guillett commented Apr 7, 2021

MattiSG Apr 7, 2021

MattiSG left a comment

guillett commented Jun 18, 2021

MattiSG left a comment

MattiSG Jun 20, 2021

MattiSG commented Jul 8, 2021

QuentinMadura commented Jul 9, 2021

guillett commented Jul 29, 2021

Ajoute une première version du dispositif départ 18-25 #1507

Ajoute une première version du dispositif départ 18-25 #1507

Conversation

guillett commented Mar 30, 2021

MattiSG left a comment

Choose a reason for hiding this comment

guillett commented Apr 7, 2021

MattiSG Apr 7, 2021

Choose a reason for hiding this comment

MattiSG left a comment

Choose a reason for hiding this comment

guillett commented Jun 18, 2021

MattiSG left a comment

Choose a reason for hiding this comment

MattiSG Jun 20, 2021

Choose a reason for hiding this comment

MattiSG commented Jul 8, 2021

QuentinMadura commented Jul 9, 2021

guillett commented Jul 29, 2021