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

Ajoute des tests sur le domaine impôts #1198

Merged
merged 15 commits into from
Nov 12, 2018
Merged

Ajoute des tests sur le domaine impôts #1198

merged 15 commits into from
Nov 12, 2018

Conversation

Morendil
Copy link
Contributor

@Morendil Morendil commented Oct 31, 2018

Merci de contribuer à OpenFisca ! Effacez cette ligne ainsi que, pour chaque ligne ci-dessous, les cas ne correspondant pas à votre contribution :)

  • Ajout de tests.
  • Détails :
    • Ajout de tests concernant le domaine impôts.

Quelques conseils à prendre en compte :

Et surtout, n'hésitez pas à demander de l'aide ! :)

@Morendil
Copy link
Contributor Author

@claireleroy Bon apparemment il y a des KO qui n'étaient pas apparus dans la première fournée… ça n'a pas l'air méchant (parfois des erreurs de l'ordre de à peine plus de 1 euro). On repart pour un tour ?

@Morendil
Copy link
Contributor Author

Morendil commented Oct 31, 2018

@claireleroy J'ai peut-être mal géré mon rebase et intégré l'ancienne version de certains de ces tests, je vais vérifier ça d'abord. (Après vérification: non il n'y a pas eu de recouvrement entre les fichiers de la première PR et ceux-ci.)

@Morendil
Copy link
Contributor Author

En regardant les logs Circle CI ces tests ajoutent à peu près 50 secondes au build (+20%)… c'est quand même assez important.

@Morendil
Copy link
Contributor Author

Morendil commented Oct 31, 2018

Bon je pense que j'ai juste mal géré la liste des tests en échec la première fois…

Pour regime_benef_reel_BIC_location_avec_cga.yaml il semble que c'est parce que rpns_individu dans ir.py ne tient pas compte de alnp_defs. (Plus généralement il y a un tas de variables dans rpns_individu qui ne sont pas utilisées, comme le montrent les lignes avec des noqa.)

Pour reduc_malraux je n'ai pas la moindre idée.

Pour les écarts de 1 euros et quelques j'ai l'impression que c'est une question de règles d'arrondi, il faudrait arrondir ce qui est renvoyé par reductions ou alors arrondir les réductions indviduellement. Si c'est bien ça, on pourrait attendre que #1195 soit mergée, ça éviterait de copier-coller 15 fois la même modification pour insérer une règle d'arrondi.

@Morendil
Copy link
Contributor Author

Morendil commented Nov 1, 2018

J'ai testé en appliquant une règle d'arrondi à l'euro inférieur (trunc) sur toutes les réductions d'impôt (individuelles, avant de les sommer) et cela nous ramène de 15 à seulement 7 tests KO, je pense que c'est comme ça que calcule la DGFIP.

Un autre test échoue pour des raisons qui n'ont rien à voir avec les réductions, mais en fait à cause d'un écart entre notre calcul de l'impôt brut sur les revenus de 2016, qui ressort à 42945.91 pour un revenu net imposable de 137817, alors que le résultat enregistré dans les tests générés est de 42947 (en réintégrant les réductions). Le calcul d'OpenFisca semble correct au regard du barème.

La formule simplifiée correspondant à cette tranche de revenu est (R x 0,41) - (13 559,06 x N) et on a bien (0.41 * 137817) - 13559.06 == 42945.91.

Quand je teste manuellement avec le simulateur j'obtiens bien un impôt brut de 42946 ce qui correspond bien au calcul d'OpenFisca avec un arrondi au plus proche (round). C'est donc le test généré qui était faux (reduc_scelli_metro_bbc.yaml en 2016). @claireleroy peut-être une erreur dans tes scripts de génération des tests ?

Ou alors ce sont les réductions qui sont mal calculées et qui devraient être un euro au-dessus…

@Morendil
Copy link
Contributor Author

Morendil commented Nov 1, 2018

Grr, il faut se méfier de absolute_error_margin. On obtient effectivement 2111 pour la réduction Scellier au lieu de 2110. La différence semble venir là aussi de cumul de sommes inférieures à 1 euro qui devraient être annulées par arrondi.

@claireleroy Si ça te va je te propose de prendre ce qui concerne les arrondis (pour uniformiser les calculs pas besoin de comprendre trop ce que font les formules) et je te laisse traiter les écarts plus importants qui relèvent sans doute de variables non traitées, tu as plus de billes que moi.

@claireleroy
Copy link
Contributor

@Morendil J'ai corrigé les tests qui ne fonctionnaient pas :

  • Pour les tests regime_benef_reel_BIC_location_avec_cga, le problème venait en effet du fait qu'on ne prenne pas en compte les déficits des locations meublées non professionnelles (alnp_defs)
  • Pour le test reduction_malraux, il n'y avait pas d'erreur mais simplement un changement en 2017 dans le périmètre de la variable représentant la réduction Malraux que l'on récuperait du site de la DGFiP. Tout le reste du calcul de l'impôt était bon, c'est donc bien une erreur seulement due à des changements de nomenclature DGFiP

Du coup je te laisse donc gérer les quelques tests restants qui ne passent pas à cause de problèmes d'arrondis, et je pense qu'on sera bon à merger :)

@claireleroy claireleroy merged commit 9c98e91 into master Nov 12, 2018
@bonjourmauko bonjourmauko mentioned this pull request Nov 12, 2018
7 tasks
@bonjourmauko bonjourmauko deleted the tests-fiscalite branch November 13, 2018 09:01
@Morendil
Copy link
Contributor Author

Morendil commented Nov 14, 2018

Une petite note post-merge pour expliquer ce qui pourrait sembler une bizarrerie dans les formules de la réduction Pinel: les règles d'arrondi appliquées par la DGFIP ont bien changé entre 2017 et 2018 (revenus de 2016 et 2017 respectivement).

Pour le mettre en évidence, une version minimale du test sur la réduction Pinel consiste à déclarer une personne seule avec des revenus (50K€ par exemple) et déclarer 10000€ dans les cases QK et QL des deux déclarations. Pour les mêmes taux et les mêmes règles métier, on obtient une réduction de 705€ dans un cas et 706€ dans l'autre.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants