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

Modification de la zone 2 en 1 pour la ville Trilport #1651

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mtifarine
Copy link
Contributor

@mtifarine mtifarine commented Aug 18, 2021

  • Changement mineur.
  • Périodes concernées : à partir du 01/07/2022.
  • Zones impactées : openfisca_france/assets/apl/20200701_zonage.csv.
  • Détails :

@mtifarine mtifarine added the contrib:msa Identification des sujets MSA label Aug 18, 2021
@mtifarine mtifarine assigned mtifarine and sandcha and unassigned mtifarine Aug 18, 2021
@benjello
Copy link
Member

@mtifarine : le changement de zone est-il récent ?

@benjello
Copy link
Member

Par ailleurs pour les tests ne pourrait-on élargir la période testée (en datant les output si c'est possible @sandcha ?) plutôt qu'en restreignant de facto les tests ?

Copy link
Contributor

@sandcha sandcha left a comment

Choose a reason for hiding this comment

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

Par ailleurs pour les tests ne pourrait-on élargir la période testée (en datant les output si c'est possible @sandcha ?) plutôt qu'en restreignant de facto les tests ?

Par élargissement de la période je comprends qu'il s'agirait de conserver la vérification des outputs en 2021-04 (supprimée pour le moment) et d'y ajouter 2021-01 (nouveauté de cette PR). Est-ce bien ce que tu proposes @benjello ? Et est-ce pour ne pas perdre une date de vérification récente ?

Si c'est techniquement possible en ajoutant les dates vérifiées aux outputs et en élargissant la période de validité des inputs (avec par exemple aide_logement: month:2020-10:7: 120 * 7 au dernier test apprenti de tests/formulas/rsa.yaml), il me semble que nous perdrions en lisibilité et en précision du calcul. En parallèle, comme montant_de_base_du_rsa est revalorisé dans cette PR en avril 2021 et employé dans les bases ressources, il me semble qu'il serait plus intéressant de garder les dates initiales des tests apprenti, c'est à dire, avril 2021.

@@ -31988,7 +31988,7 @@ CODGEO,LIBCOM_2014,REG,DEP,Zonage
77472,La Trétoire,11,77,2
77473,Treuzy-Levelay,11,77,2
77474,Trilbardou,11,77,1
77475,Trilport,11,77,2
77475,Trilport,11,77,1
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 C'est effectivement ce qu'indique le simulateur service-public.

Copy link
Contributor

Choose a reason for hiding this comment

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

S'il s'agit d'une mise à jour de zonage, le nom du fichier CSV ne devrait-il pas aussi être mis à jour ? 🤔

Si on voulait identifier le sens de la date donnée dans le nom du fichier (14 septembre 2011), il me semble qu'il faudrait définir s'il s'agit simplement de sa date de création ou d'une date officielle de publication du zonage.

Cela ne semble en tout cas pas être celle de l'arrêté initial qui ne contient pas les informations par commune. Du coup, à toi @cbenz qui a créé ce fichier il y a un temps certain, te souviens-tu du sens de cette date et/ou de ta référence alors ? ^^

Le assets/apl/README.md évoque bien une publication à venir sur le site d'etalab mais je ne ne la trouve pas (il y a néanmoins les données du simulateur service-public).

@sandcha
Copy link
Contributor

sandcha commented Aug 19, 2021

le changement de zone est-il récent ?

Le changement ne serait-il pas lié à l'action en justice évoquée dans cet article ?

Copy link
Contributor

@sandcha sandcha left a comment

Choose a reason for hiding this comment

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

Merci @mtifarine pour la correction de zone et ces revalorisations !
Cette demande de modification concerne une revalorisation qui n'a plus lieu d'être depuis le déplacement d'un paramètre et quelques références qui sont proposées à l'ajout.

Copy link
Contributor

@sandcha sandcha left a comment

Choose a reason for hiding this comment

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

@mtifarine Il n'y a pas de changement sur cette PR depuis la dernière revue. Auriez-vous des commits en local qui n'auraient pas été poussés en remote ? 🤔

@frtomas
Copy link
Contributor

frtomas commented Sep 27, 2021

@sandcha les 2 commits qui ne concernaient pas Trilport était une petite erreur sur cette branche, ils font partie de l'autre PR

Nous les avons retiré, et sans ces 2 commits superflus, il me semble qu'il n'y a plus d'autre points de discussion sur cette PR.

Est-ce que j'en ai raté un ?

@frtomas frtomas dismissed sandcha’s stale review October 6, 2021 08:13

Comme précisé en commentaire, la remarque concernant les revalorisation porte sur des commit superflus qui ont été retiré de cette PR

@tomy-isabelle
Copy link

bonjour @benjello , @sandcha , @mtifarine , @frtomas
dans la mesure où il n'y a plus de discussion sur cette PR
est il possible de la merger s'il vous plait ?
merci

@guillett
Copy link
Member

Bonjour @tomy-isabelle est ce que cette modification correspond à une correction ou bien à une évolution de la zone ? S'il s'agit d'une évolution, à quelle date faudrait-il l'associer ? Faute d'avoir une historisation de ce paramètre nous pourrions au moins indiquer ça dans le changelog et sur GitHub.

@tomy-isabelle
Copy link

bonjour, ci-joint référence qui justifie l'évolution de zone :

Source :
La commune de Trilport (77) est reclassée de zone II en zone I à partir du 1 juillet 2020

Texte : Arrêté du 27 mai 2020 modifiant l'arrêté du 17 mars 1978 modifié relatif au classement des communes par zones géographiques

@sandcha
Copy link
Contributor

sandcha commented Jun 29, 2023

Merci @robinguill @frtomas pour la mise au clair de cette PR 🙌

La règle de nommage du fichier de zonage n'est pas explicite (cf. cet ancien commentaire de vérification) mais je vous propose d'en profiter pour définir une règle et renommer 20110914_zonage.csv (où 2011 ressemble à l'année des débuts d'openfisca) en 20200701_zonage.csv où le 1er juillet 2020 correspond à la date d'entrée en application de ce zonage. Seriez-vous d'accord pour faire ça ?

Au merge de cette PR, les calculs d'aides au logement antérieurs au 01/07/2020 seront erronés pour Trilport. L'issue a été remontée ici : #2136

@robinguill
Copy link
Contributor

robinguill commented Jul 11, 2023

@sandcha avec le renommage pour traçage de la modification du zonage, peut être pouvons nous approuver les modifications ?
Ou faut il trouver un meilleur moyen de tracer ces changements ?

@robinguill robinguill requested review from sandcha and removed request for sandcha October 3, 2023 13:08
@frtomas
Copy link
Contributor

frtomas commented Oct 24, 2023

@sandcha @benjello @guillett tout les points de cette PR ayant été reglés, peut-elle effectivement être mergée ?
Quelle type de montée de version conseilleriez vous pour ce type de modification portant sur le zonage, qui a priori ne pose pas de problème de compatibilité mais qui n'est pas anodin ?

@benjello
Copy link
Member

Cela peut être vu comme une correction de bug, car le résultat était erroné.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants