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

Application d'un barème spécifique à certains publics "accession" - Aides au logement (FAM_Reforme_Accession) #1138

Merged
merged 4 commits into from
Oct 3, 2018

Conversation

mtifarine
Copy link
Contributor

  • Évolution du système socio-fiscal.
  • Périodes concernées : toutes.
  • Zones impactées :
    • model\prestations\aides_logement.
  • Détails :
    • Suppression des aides au logement dans le cadre de l'accession à la propriété, sauf exception.
    • Ajout des variables:
    • TypeEtatLogement
    • etat_logement
    • aide_logement_date_pret_conventionne
    • aides_logement_primo_accedant_eligibilite

@mtifarine mtifarine added the contrib:msa Identification des sujets MSA label Oct 1, 2018
@mtifarine mtifarine requested a review from frtomas October 1, 2018 11:50

est_zone_3 = (zone_apl == TypesZoneApl.zone_3)

date_pret_conventionne_avant_2018_01 = (aide_logement_date_pret_conventionne < date(2018, 1, 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

Je me demande si c'est vraiment utile de chercher à calculer l'impact de cette période transitoire très précise entre le 1er janvier et le 31 janvier 2018, au prix d'une formule plus compliquée.

Est-ce que cette formule va être utilisée directement par un outil de simulation?

Si oui, pourquoi est-ce que cette formule ne tient pas compte de l'autre condition d'application, à savoir une date de demande du prêt antérieure au 31 décembre 2017 ?

Si non, serait-il acceptable de simplifier la formule en ne retenant que la date du 1er janvier ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merci @Morendil,
J'ai posé la question à @JenniferTelep pour cette remarque.
En attendant sa réponse, j'ai apporté des modifications selon tes remarques.

date_pret_conventionne_avant_2018_01 = (aide_logement_date_pret_conventionne < date(2018, 1, 1))
date_pret_conventionne_avant_2018_02 = (aide_logement_date_pret_conventionne < date(2018, 2, 1))
date_pret_conventionne_avant_2020_01 = (aide_logement_date_pret_conventionne < date(2020, 1, 1))
date_pret_conventionne_apres_2020_01 = (aide_logement_date_pret_conventionne >= date(2020, 1, 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

Si on simplifie comme suggéré ci-dessus on se retrouve avec seulement trois cas:

  • prêt conclu avant le 1er janvier 2018
  • prêt conclu en zone 3 sur logement ancien avant le 1er janvier 2020
  • tout autre cas

…et on pourrait utiliser la clause "par défaut" du select() de numpy pour la condition par défaut (False), évitant ainsi la redite entre les conditions "avant 2020" et "après 2020".

@@ -1089,6 +1152,18 @@ def formula_2007_07(famille, period, parameters):

return (L > 0) * K * max_(0, (L + C - Lo))

def formula_2018_01(famille, period, parameters):
Copy link
Contributor

@Morendil Morendil Oct 1, 2018

Choose a reason for hiding this comment

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

Il est inutile d'introduire une nouvelle formule datée, puisque la formule pour l'éligibilité couvre toutes les périodes. Cela introduit de la duplication sans aucun bénéfice. Il suffit de modifier la méthode formula_2007_07 directement avec le code ici présent (formula_2018_01). Tous les tests continuent à passer. (D'ailleurs ils continuent à passer même si cette formule n'est pas datée, et qu'on nomme simplement la méthode formula edit j'ai parlé trop vite, il y a un unique effet de bord sur la CAAH.)

output_variables:
aides_logement_primo_accedant_eligibilite: 1

- name: "cas non passant 1"
Copy link
Contributor

Choose a reason for hiding this comment

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

J'éviterais l'intitulé "non passant" qui suggère que le test est… non passant, alors que par construction tous les tests de l'intégration continue doivent rester au vert (passants).

Ce serait plus utile à des fins de documentation de décrire les cas de test par des intitulés comme "Eligible - prêt conclu avant le 1er janvier 2018 et la suppression de l'APL primo-accédant", ou "Non éligible suite à suppression de l'APL primo-accédant - prêt conclu en zone 3 pour l'acquisition d'un logement neuf"

@@ -0,0 +1,195 @@
- name: "cas passant 1"
description: Nombre de personnes à charge pour les AL
Copy link
Contributor

Choose a reason for hiding this comment

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

Cette description ne correspond pas au contenu du test ?

@mtifarine mtifarine force-pushed the msa_fam_reforme_accession branch 2 times, most recently from d4eb25e to de5534b Compare October 2, 2018 14:14
@mtifarine
Copy link
Contributor Author

@Morendil
Lors de la vérification de mon push, il détecte des erreurs de style (espace, indentation, etc.) dans des lignes où je ne l'ai pas touché.

@mtifarine mtifarine requested a review from Morendil October 2, 2018 14:57
@Morendil
Copy link
Contributor

Morendil commented Oct 2, 2018

@mtifarine Oui, on souhaite converger vers un style de code plus standardisé, depuis #1135 on va effectuer cette vérification sur les nouveaux fichiers. L'intention est de lisser le travail de mise en conformité dans le temps, plutôt que tout changer d'un coup.

Je prends la mise au standard si tu le souhaites (ça me permettra une passe de relecture), pour les futures PR il y aura des corrections du même type à faire au fil de l'eau.

@mtifarine
Copy link
Contributor Author

@Morendil tu peux pendre la mise au standard des fichiers de cette PR.
Et tu peux nous fournir si t'as les règles de style à suivre qui nous aidera prochainement pour la mise au standard pour les futures PR.

@Morendil
Copy link
Contributor

Morendil commented Oct 3, 2018

Tout est là je crois: https://lintlyci.github.io/Flake8Rules/

@benjello
Copy link
Member

benjello commented Oct 3, 2018

@mtifarine @Morendil : il y a deux règles non-standard que l'on suit.

  • espace autour du signe égal même pour les arguments par défaut des fonctions
  • Délimiteur fermant au même niveau d'indentation que le dernier élément quand il n'est pas sur la même ligne que celui-ci.
ma_liste = [
    1,
    2,
    ]

@Morendil
Copy link
Contributor

Morendil commented Oct 3, 2018

@mtifarine Une remarque, je mets à jour le Changelog et je constate que le lien (après le numéro de version) était vers l'issue #931 et pas vers la PR #1138, c'était volontaire ? J'avais compris que c'était le numéro des PR et non des issues qu'on indiquait.

J'ai aussi ajouté une annotation ci-dessus dans le descriptif de la PR pour que l'issue #931 soit automatiquement fermée quand on mergera la PR.

@mtifarine mtifarine force-pushed the msa_fam_reforme_accession branch from de5534b to 8d00548 Compare October 3, 2018 09:02
@Morendil
Copy link
Contributor

Morendil commented Oct 3, 2018

@mtifarine Pas besoin de corriger, je suis dessus!

@Morendil Morendil force-pushed the msa_fam_reforme_accession branch from 549c95a to 6fc74b2 Compare October 3, 2018 09:27
@Morendil Morendil merged commit f375f6c into master Oct 3, 2018
@Morendil Morendil deleted the msa_fam_reforme_accession branch October 3, 2018 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contrib:msa Identification des sujets MSA kind:evolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants