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

Make sure an AAH eligible individual is NOT ASS eligible #784

Merged
merged 2 commits into from
Jul 19, 2017
Merged

Conversation

guillett
Copy link
Member

@guillett
Copy link
Member Author

I duplicated some code in that PR. Would it be better to have something like:

def formula_2017_01_01(individu, period):
    aah_eligible = individu('aah_eligible', period)
    return _and(formula(individu, period), _not(aah_eligible))

A test has been created in MesAides test DB, I will move it into the OpenFisca test files.

@MattiSG MattiSG requested review from benjello and removed request for MattiSG June 27, 2017 15:53
Copy link
Member

@benjello benjello left a comment

Choose a reason for hiding this comment

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

Changements cosmétiques.
Merci !

# 1 si demandeur d'emploi
activite = individu('activite', period)

# Indique que l'user a travaillé 5 ans au cours des 10 dernieres années.
Copy link
Member

Choose a reason for hiding this comment

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

Qui est l'user ? ;-)

# Indique que l'user a travaillé 5 ans au cours des 10 dernieres années.
ass_precondition_remplie = individu('ass_precondition_remplie', period)

are_perceived_this_month = individu('chomage_net', period)
Copy link
Member

Choose a reason for hiding this comment

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

Utiliser des noms de variables français quand il s'agit de coder la législation

@guillett
Copy link
Member Author

@benjello que penses-tu de l'approche suggérée dans mon commentaire ? Je n'aime pas trop l'idée d'avoir autant de code dupliqué.

Merci.

@benjello
Copy link
Member

La solution proposée est pire que le problème soulevé selon moi. Si tu veux éviter de dupliquer du code, il vaut mieux écrire une helper function qui évite cette duplication tout en laissant le code lisible.


return and_(and_(and_(not_(aah_eligible), activite == 1), ass_precondition_remplie), are_perceived_this_month == 0)
return and_(not_(aah_eligible), and_(and_(activite == 1), ass_precondition_remplie), chomage_percu_ce_mois == 0)
Copy link
Member

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 y a un problème de parenthèses.
Et je définirais la variable

demandeur_emploi_non_indemnise = and_(individu('activite', period) == 1, individu('chomage_net', period) == 0)

pour réutilisation afin d'être plus clair tout en limitant l'empreinte mémoire

Copy link
Member Author

Choose a reason for hiding this comment

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

@benjello, merci beaucoup pour tes commentaires.

Désolé pour le bruit, je ne m'attendais pas à ta revue n'ayant pas à nouveau request review.

J'ai tendance à push beaucoup sur GH pour éviter de n'avoir qu'une version en local.

Copy link
Member

Choose a reason for hiding this comment

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

@guillett: à ton service !

@guillett guillett removed the request for review from fpagnoux June 30, 2017 09:59
@guillett
Copy link
Member Author

@benjello ⚠️ je vais rebaser pour me débarrasser des problèmes de CI.

Copy link
Member

@benjello benjello left a comment

Choose a reason for hiding this comment

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

Minor change and ok

def formula_2017_01_01(individu, period):
aah_eligible = individu('aah_eligible', period)

# 1 si demandeur d'emploi
Copy link
Member

Choose a reason for hiding this comment

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

Commentaire inutile

Copy link
Member Author

Choose a reason for hiding this comment

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

Tu trouves vraiment que c'est inutile ?
Honnêtement individu('activite', period) == 1 se trouve clarifier avec ce commentaire selon moi.

Copy link
Member

Choose a reason for hiding this comment

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

Alors être complètement explicite activite = 1 pour un demandeur d'emploi

CHANGELOG.md Outdated

* Évolution du système socio-fiscal.
* Périodes concernées : à partir du 01/01/2017
* Zones impactées : ` openfisca_france/model/prestations/minima_sociaux/ass.py`.
Copy link
Member

@fpagnoux fpagnoux Jul 4, 2017

Choose a reason for hiding this comment

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

/prestations/minima_sociaux/ass

Copy link
Member

Choose a reason for hiding this comment

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

The normalized format doesn't include the .py extension, nor the openfisca_france/model prefix


return and_(and_(activite == 1, ass_precondition_remplie), are_perceived_this_month == 0)
def formula_2017_01_01(individu, period):
aah_eligible = individu('aah_eligible', period)
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi aah_eligible plutôt que aah tout court ?

Copy link
Member

Choose a reason for hiding this comment

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

Si je suis en situation de handicap avec un taux d'incapacité de 50%, mais que les médecins de la MDPH estiment que je suis malgré tout apte à travailler, aah_eligible est vraie pour moi, mais je n'aurais pas de aah.
Pour coller au plus près à ce qui est dans la loi (non-cumulabilité de l'AAH et de l'ASS), il faudrait à mon avis mettre aah ici.

Copy link
Member

Choose a reason for hiding this comment

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

Bien vu @fpagnoux. Prendre aah > 0.

@guillett
Copy link
Member Author

@benjello ayant fait les modifications suggérées par @fpagnoux. Peux-tu ou puis-je merger cette PR ? Merci.

@benjello
Copy link
Member

@guillett : GTM (après résolution des conflits liés au Bump)

@guillett guillett merged commit 15b050a into master Jul 19, 2017
@guillett guillett deleted the aah_ass branch July 19, 2017 08:14
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.

4 participants