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

Mets à jour la réduction d'impôt "Pinel" pour 2018 (IR 2019 sur revenus 2018) #1325

Merged
merged 21 commits into from
Jun 7, 2019

Conversation

claireleroy
Copy link
Contributor

@claireleroy claireleroy commented May 22, 2019

  • Évolution du système socio-fiscal.
  • Périodes concernées : à partir du 01/01/2018.
  • Zones impactées : model/prelevements_obligatoires/impot_revenu/.
  • Détails :
    • Ajoute comme inputs variables les nouvelles cases de la déclaration IR 2018 liée à la réduction Pinel.
    • Renomme les anciennes inputs variables du même nom, selon la méthode habituelle
    • Mets à jour la formule de calcul de la réduction (introduction des investissements réalisés en 2018 et ajout du report des investissements 2017)
    • Factorise les formules de la réduction Pinel
    • Renomme le paramètre seuilen plafond
    • Ajoute 2 tests issus des résultats du calculateur en ligne du site impots.gouv

Cf. la brochure pratique pour plus de détails sur le fonctionnement de cette réduction.


Ces changements :

  • Modifient l'API publique d'OpenFisca France (par exemple renommage ou suppression de variables).
  • Ajoutent une fonctionnalité (par exemple ajout d'une variable).
  • Corrigent ou améliorent un calcul déjà existant.

@claireleroy claireleroy requested review from a team May 23, 2019 08:13
@claireleroy claireleroy changed the title Mets à jour la réduction d'impôt "Pinel" pour 2018 Mets à jour la réduction d'impôt "Pinel" pour 2018 (IR 2019 sur revenus 2018) May 23, 2019
@Morendil
Copy link
Contributor

@claireleroy Vous avez besoin de cette mise à jour pour une étude? Ou c'est seulement par souci de le mettre à jour? Dans le 2nd cas je suis plutôt tenté de ne pas merger la PR dans son état actuel et plutôt de mettre la priorité sur une solution à #853 de façon à ne pas continuer à produire les formules de l'année N en copiant-collant celles de l'année N-1.

@claireleroy
Copy link
Contributor Author

claireleroy commented May 24, 2019

@Morendil Oui on va avoir besoin de mettre à jour toute la partie sur l'impôt sur le revenu dans le cadre de plusieurs études qui utilisent l'IR 2019 sur revenus 2018 (qui vont se dérouler de juin à octobre). Je suis pour continuer l'update de l'IR de cette façon mais je veux bien me pencher avec toi ou d'autres gens sur comment régler ce problème à un moment plus calme pour nous (cet automne par exemple). Qu'en dis tu ?

@Morendil
Copy link
Contributor

@claireleroy Je m'inquiète de l'éventualité que la mise à jour de nombreux dispositifs se fasse avec autant de duplication à chaque fois, tu as une idée du nombre de formules fiscales qu'il faut s'attendre à voir mettre à jour sur ce mode dans les semaines à venir?

@claireleroy
Copy link
Contributor Author

@Morendil une dizaine je dirais... je comprends l'inquiétude mais bon je pense que dans tous les cas factoriser ces formules va être un gros chantier (déjà plus de 8000 lignes de codes pour seulement reductions.py et credits_impot.py donc finalement quelques centaines de ligne en plus c'est conséquent mais ça ne changera pas tant que ça l'ampleur du problème à gérer non ?

Par ailleurs, si pour cette PR tu vois une façon d'updater sans dupliquer je prends! Peut-être qu'avec les end date etc. ça peut marcher je ne me suis pas penchée là-dessus. Je n'ai juste pas le temps de tout résoudre maintenant, plutôt à l'automne.

@Morendil
Copy link
Contributor

@claireleroy Oui il y a moyen d'améliorer la formulation de ces dispositifs, on avait commencé avec #1205 ou #1213. C'était sur le dernier trimestre de 2018 et je me souviens justement qu'on avait abandonné ces travaux notamment parce qu'il était difficile de programmer du temps pour travailler ensemble et construire ainsi un consensus. Je suis donc dubitatif sur l'idée de repousser ça encore à l'automne qui vient. Je te proposerais bien, plutôt, qu'on prévoie un atelier (à Ségur ou à l'IPP) pour travailler ensemble sur ces formules maintenant que tu es dessus, et transmettre des techniques de factorisation que tu pourras ensuite appliquer en autonomie. Qu'en penses-tu? Cette semaine on a trop peu de temps avec le Grand Séminaire mais à partir de la semaine prochaine c'est faisable.

@claireleroy
Copy link
Contributor Author

@Morendil Ok, disponible la semaine prochaine à l'IPP pour regarder cela !

Je ne peux pas garantir que ce sera appliqué car la période juin-octobre est une période chargée côté IPP et on doit prioriser les choses (on sera peut-être amené à faire notre propre branche IPP côté CASD) mais dans tous les cas voyons ensemble quelle est la solution optimale et à quel horizon elle pourrait être implémentée :)

@claireleroy claireleroy force-pushed the update-pinel branch 11 times, most recently from b23d144 to d915e5c Compare June 5, 2019 13:40
@claireleroy
Copy link
Contributor Author

@Morendil @sandcha J'ai finalement factorisé toutes les années car c'était court dans le cas Pinel. J'imagine qu'on pourrait aller plus loin en factorisant toutes ces formules en une seule et en jouant sur les années sur lesquelles on boucle. Si vous voyez comment le faire concrètement, je suis preneuse ! Sinon il me semble que la PR est prête pour review ?

@Morendil
Copy link
Contributor

Morendil commented Jun 7, 2019

Merci @claireleroy c'est top! J'ai poussé jusqu'au dernier cran en regroupant toutes les formules en une seule (sauf 2014), il suffit de calculer l'année fiscale de référence et travailler sur des range.

Résultat, le bilan en lignes de code de cette PR est très faible, un gain net de 86 lignes tout compris et une diminution si on exclut les nouveaux tests. Je pense que ça nous simplifiera la vie aussi pour les années à venir (il va falloir gérer les fins de période d'investissement mais c'est un autre exercice).

Un petit détail à corriger signalé par l'échec des tests sur la version factorisée: on ne sait pas encore calculer la réduction pour l'année 2019. Le test concerné test_basics était précédemment passant pour l'année 2019 mais à mon avis il renvoyait des résultats faux pour cette année.

Je me demande comment le gérer, on peut ajouter un end à la variable rpinel mais il faudra alors ajouter un commentaire pour préciser que ce n'est pas la date de fin du dispositif mais un garde-fou pour éviter que le calcul soit demandé sur des années où on ne peut pas l'effectuer faute de connaître la nomenclature de la déclaration. Qu'en penses-tu ?

@claireleroy
Copy link
Contributor Author

@Morendil Super c'est très propre comme cela!

Pour les tests sur l'année 2019, je pense que ça révèle un truc assez embêtant qu'il faut gérer avant de merger cette PR :

  • Aujourd'hui, si j'ai une variable avec une formule datée pour 2017_01_01 et 2018_01_01 disons, elle renvoie bien une valeur pour l'année 2019 (et ce avec la formule 2018). C'est le fonctionnement normal d'OpenFisca. Seul un attribut end peut venir modifier ce comportement.
  • Ce comportement est très utile (en tout cas pour nous à l'IPP) . Évidemment, dans certains cas, comme les variables dont la formule change chaque année (ex : Pinel), cela peut impliquer que la valeur renvoyée pour les années à venir est "fausse". Mais cela vaut alors pour tout OpenFisca. Ex : je calcule le RSA en 2025, ce sera calculé avec la dernière formule connue, or il y aura peut une réforme du RSA d'ici là et ma valeur est donc fausse.
  • Ici, il faut donc corriger la formule de la réduction Pinel. Elle bugge pour les années au-delà de 2018 car on boucle sur les années qui doivent être des keys de cases_report et cases_investissement. A mon sens, il faudrait que la réduction soit calculée pour toutes années supérieures ou égales à 2014 et que la formule gère l'absence de cases pour les années au-delà de 2019 (absence = 0). Je ne suis pas favorable à des end garde-fous mais à une adaptation de la formule.

Je peux essayer de regarder cela.

@Morendil
Copy link
Contributor

Morendil commented Jun 7, 2019

@claireleroy On peut toujours ajouter dans la formule une vérification de l'existence de annee_fiscale dans le tableau des cases, et renvoyer 0 si on ne la trouve pas.

@claireleroy
Copy link
Contributor Author

@Morendil oui ça me parait faire l'affaire!

@claireleroy claireleroy merged commit c5adcfc into master Jun 7, 2019
@Morendil
Copy link
Contributor

Morendil commented Jun 7, 2019

@claireleroy Ton Changelog et ton numéro de versions ne sont pas cohérents: tu signales cette version comme un BREAKING mais tu ne fournis pas d'indication sur ce qui doit faire l'objet d'une migration.

Pour moi ce n'est pas un BREAKING mais un MINOR, soit 42.6.1, sauf si j'ai loupé quelque chose qui rend cette mise à jour non rétro-compatible ?

Par ailleurs le niveau du header markdown devant le numéro de version dans le Changelog est lié avec le type de montée de version, niveau 3 (###) pour un PATCH, niveau 2 (##) pour un MINOR et niveau 1 pour un MAJOR (#).

Par ailleurs je ne retrouve pas l'effet de mes modifs en termes de nombre de lignes de code, lorsque j'ai pushé mon commit on était retombés à +86 lignes net et on est maintenant à +278, j'ai l'impression qu'une de nos manips a réintroduit les versions successives de la formule rpinel.

Je te suggère de faire un revert de la PR et de repartir du commit a5501f4

@claireleroy
Copy link
Contributor Author

@Morendil Oui désolée tu as raison, j'ai du me louper lors du rebase.... Comment fait-on un revert d'une PR ?

Pour le Changelog, je pensais que toute PR qui renommait ou supprimait des variables était un breaking change ? Si oui, il faut effectivement que j'explicite comment migrer.

@Morendil
Copy link
Contributor

Morendil commented Jun 7, 2019

toute PR qui renommait ou supprimait des variables

Ah oui les cases de l'impôt renommées… va pour une MAJOR alors

@Morendil
Copy link
Contributor

Morendil commented Jun 7, 2019

Comment fait-on un revert d'une PR ?

Tu as un bouton en bas à droite du fil d'évènements de la PR… On ne va pas merger en urgence le revert, ça peut attendre la semaine prochaine. Mais tu peux créer la PR de rollback maintenant, comme ça on sera tous au courant que c'est en cours?

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.

2 participants