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

Reform dynamique : ajoute la gestion de la condition attached_to_institution #191

Merged
merged 5 commits into from
Nov 27, 2023

Conversation

Allan-CodeWorks
Copy link
Contributor

Description

La condition attached_to_instititution n'as pas été développée jusqu'a maintenant car elle n'est pas explicitée dans le fichier yaml d'une aide.
Cette condition est une condition géographique dont le périmètre est régi par l'institution en question.
Les aides avec cette condition ont un champ institution dans lequel réside une chaine de caractères qui correspond a nom d'un fichier dans un autre répertoire sans leur extension .yml (pas compatible avec .yaml pour le moment)

Cette particularité technique implique que la condition ne peut pas être traitée comme les autres car il faudrait intégrer toutes les institutions dans openfisca, ce qui ne semble pas souhaitable.

Cette PR propose donc d'agir avant l'importation des aides dans le tax and benefit system, juste après avoir lu les fichiers d'aides et les avoir transformés en dict, on :

  • supprime la condition attached_to_institution des conditions_generales
  • lit le fichier de l'institution correspondante grâce au champ institution
  • ajoute une condition géographique équivalente aux informations du fichier institution dans les conditions générales

Modifié en même temps :

Dans le boy scout spirit je me suis dit qu'il était ok de faire quelques changements :

  • simplification de la gestion des path par défaut
  • changement du type d'erreur remontée en cas de condition ou profil inconnu, on passe de KeyError à NotImplementedError

Added

  • Ajoute la gestion de la condition attaches_to_institution

@Allan-CodeWorks Allan-CodeWorks requested a review from a team November 21, 2023 13:56
@Allan-CodeWorks Allan-CodeWorks self-assigned this Nov 21, 2023
@Allan-CodeWorks Allan-CodeWorks force-pushed the reform_dynamique_attached_to_institution branch from a0a872c to e124d1a Compare November 21, 2023 14:20
Copy link
Contributor

@baptou12 baptou12 left a comment

Choose a reason for hiding this comment

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

Je te l'ai review comme si c'était good commit par commit, si tu veux les changer.

Sinon on peut ignorer.

openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Outdated Show resolved Hide resolved
openfisca_france_local/aides_jeunes_reform.py Show resolved Hide resolved
@Allan-CodeWorks Allan-CodeWorks force-pushed the reform_dynamique_attached_to_institution branch 4 times, most recently from 7be7351 to 59b7355 Compare November 23, 2023 10:15
Copy link
Contributor

@baptou12 baptou12 left a comment

Choose a reason for hiding this comment

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

Top juste la question sur les regions et une proposition de simplification de code

@Allan-CodeWorks Allan-CodeWorks force-pushed the reform_dynamique_attached_to_institution branch from 59b7355 to cd7770d Compare November 23, 2023 14:02
Copy link
Contributor

@baptou12 baptou12 left a comment

Choose a reason for hiding this comment

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

LGTM, merci pour les rebases

@Allan-CodeWorks Allan-CodeWorks force-pushed the reform_dynamique_attached_to_institution branch from cd7770d to 170d33b Compare November 27, 2023 08:57
@Allan-CodeWorks Allan-CodeWorks merged commit aca9a0d into master Nov 27, 2023
9 checks passed
@Allan-CodeWorks Allan-CodeWorks deleted the reform_dynamique_attached_to_institution branch November 27, 2023 09:04
@guillett guillett added this to the Passé milestone Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kind:evolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants