Remove fuzzy #731

michelbl · 2017-04-14T18:25:41Z

Connected to openfisca/openfisca-core#437

fpagnoux

There are a lot of redundant lines.

Given the size of the diff, it's hard to check the changes one by one. The tests will be our guarantee. To pass them, you should check this out.

fpagnoux · 2017-04-18T16:54:12Z

openfisca_france/parameters/bouclier_fiscal.xml

-    <VALUE deb="2007-01-01" fin="2010-12-31" valeur="0.5" /> # Suppression du bouclier fiscal à compter des revenus réalisés en 2011. #
-    <VALUE deb="2006-01-01" fin="2006-12-31" valeur="0.6" />
+    <END deb="2011-01-01" />
+    <VALUE deb="2007-01-01" valeur="0.5" /> # Suppression du bouclier fiscal à compter des revenus réalisés en 2011. #


This comment should be one line above, no ?

The transformation was automated. I am willing to check manually for every comment and decide to which line it naturally belongs. However, I would like to be sure that the automated regexes won't need to be modified and replayed.

fpagnoux · 2017-04-18T16:55:45Z

setup.py

@@ -7,7 +7,7 @@

 setup(
    name = 'OpenFisca-France',
-    version = '16.1.1',
+    version = '17.0.0',


Not sure this is a major version. Is there any change brought by this PR from an OpenFisca-France user point of view ?

Original openfisca-france xml code will break. Apart from that, no.

Then this should just be a minor.

fpagnoux · 2017-04-18T16:57:09Z

openfisca_france/parameters/tns.xml

-      <VALUE deb="2016-01-01" fuzzy="true" valeur="0.5" />
-      <VALUE deb="2008-01-01" fin="2015-12-31" valeur="0.5" />
+      <VALUE deb="2016-01-01" valeur="0.5" />
+      <VALUE deb="2008-01-01" valeur="0.5" />


I see a lot of this: two lines with the same values. It should be only one line.

Or should be a PLACEHOLDER as discussed

Sure, but I don't think we (the core team) should take the responsability to choose where a PLACEHOLDER is relevant and where it's not.

This is not a lot of work to check in the transformation script if consecutive values are the same or not. However the redundancy was already in the original file. IMHO this check is exceeding the core team's purpose.

This line dupplication was brought by the fuzzy. As we are getting rid of it, I think we should clean it and remove the redundant values.

A regex like :

(?: )*<VALUE deb="\d{4}-\d{2}-\d{2}" valeur="((?:\d|.)+)" \/> ((?: )*<VALUE deb="\d{4}-\d{2}-\d{2}" valeur="\1" \/>)

->

\2

should remove all the duplicates.

I can run it if you want.

Pour information, ce cas est présent 706 fois.

Je ne suis pas serein sur le fait d'automatiser la transformation par une regex. Dans le cas suivant ça introduirait un bug :

<VALUE deb="2016-01-01" valeur="0.5" /> <VALUE deb="2014-01-01" valeur="0.5" /> <VALUE deb="2015-01-01" valeur="1.0" />

@benjello et @cbenz, quel est votre avis ?

Tester si les dates sont bien ordonnées. Si c'est le seul cas dangereux ...

cbenz · 2017-04-19T08:39:07Z

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog

+# 17.0.0 - [#???](https://github.com/openfisca/openfisca-france/pull/???)


Fix ??? :)

cbenz · 2017-04-19T08:40:16Z

openfisca_france/parameters/tns.xml

-      <VALUE deb="2016-01-01" fuzzy="true" valeur="0.5" />
-      <VALUE deb="2008-01-01" fin="2015-12-31" valeur="0.5" />
+      <VALUE deb="2016-01-01" valeur="0.5" />
+      <VALUE deb="2008-01-01" valeur="0.5" />


Or should be a PLACEHOLDER as discussed

fpagnoux · 2017-04-27T17:48:14Z

openfisca_france/parameters/impot_revenu.xml

+        <VALUE deb="1967-01-01" valeur="0.5" />
+        <VALUE deb="1966-01-01" valeur="0.55" />
+        <VALUE deb="1949-01-01" valeur="0.5" />
+        <VALUE deb="1948-01-01" valeur="0.4" />
      </TAUX>
    </


@benjello est ce que tu peux stp nous éclairer sur ce point qui fait crasher les tests:

Quand on regarde le XML, il semble que la TEICAA n'existe plus depuis le 1er janvier 2013. Pourtant la formule qui l'utilise n'est pas datée. Est-ce que:

La TEICAA est encore en vigueur et il faut supprimer cette date de fin

La TEICAA n'est plus en vigueur et il faut dater la formule

Je ne suis pas spécialiste mais les cases 5QM et 5RM existent toujours et ne semblent pas avoir changé de définition depuis 2013 donc je suppose que rien n'a changé ...

Les tests ne plantaient pas avant, car les barèmes sans tranche (toutes les tranches sont finies) étaient toujours considérés comme des barèmes. Ils retournaient systématiquement 0. Maintenant, on considère qu'un barème n'existe plus dès lors qu'il n'a plus de tranche. S'il est appelé, il renvoie une exception ParameterNotFound.
Est-ce qu'il faut revenir au comportement précédent ou enlever le noeud END ?

fpagnoux · 2017-04-27T17:50:15Z

@michelbl la propriété ignore a été dépréciée pour les fichiers de test. Il faut soit adapter le test, soit le supprimer en expliquant bien les raisons qui poussent à croire que ce test est faux.

fpagnoux · 2017-04-27T17:55:02Z

Also, as discussed with @MattiSG, we should not remove the param.xml file in this PR. This is a too big issue to be secretly removed ;)

Supprime l'attribut fuzzy. Déplace l'attribut 'fin' des noeuds VALUE vers un nouveau noeud END. Cette transformation a été effectuée à l'aide du script Openfisca-Core remove_fuzzy.py.

Les absents ont toujours tort

michelbl requested review from fpagnoux and cbenz April 14, 2017 18:30

fpagnoux requested changes Apr 18, 2017

View reviewed changes

cbenz previously requested changes Apr 19, 2017

View reviewed changes

michelbl requested review from fpagnoux and cbenz April 25, 2017 17:42

fpagnoux reviewed Apr 27, 2017

View reviewed changes

fpagnoux mentioned this pull request Apr 27, 2017

Remove fuzzy openfisca/openfisca-core#488

Merged

michelbl force-pushed the remove-fuzzy branch 2 times, most recently from 8d15943 to d853b03 Compare May 3, 2017 16:28

Michel Blancard added 5 commits May 3, 2017 19:13

Supprime l'attr fuzzy des fichiers xml et adapte

ff7382b

Supprime l'attribut fuzzy. Déplace l'attribut 'fin' des noeuds VALUE vers un nouveau noeud END. Cette transformation a été effectuée à l'aide du script Openfisca-Core remove_fuzzy.py.

Supprime fuzzied.txt

b3fac26

Réécrit la modification de législation (reforms)

bd58b72

Prolonge la validité des variabes rafp et teicaa

6820fff

Enlève tehr (pas encore définie) d'un test

ffb8333

fpagnoux force-pushed the remove-fuzzy branch from d853b03 to 2a751c8 Compare May 3, 2017 17:15

Michel Blancard and others added 2 commits May 3, 2017 19:37

Changelog et numero de version

0634078

Upgrade core

0017fe5

fpagnoux force-pushed the remove-fuzzy branch from 31fb100 to 0017fe5 Compare May 3, 2017 17:38

fpagnoux approved these changes May 3, 2017

View reviewed changes

fpagnoux merged commit e53121c into master May 3, 2017

fpagnoux deleted the remove-fuzzy branch May 3, 2017 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove fuzzy #731

Remove fuzzy #731

michelbl commented Apr 14, 2017

fpagnoux left a comment

fpagnoux Apr 18, 2017

michelbl Apr 25, 2017

fpagnoux Apr 18, 2017

michelbl Apr 25, 2017

fpagnoux Apr 27, 2017

fpagnoux Apr 18, 2017

cbenz Apr 19, 2017

fpagnoux Apr 19, 2017 •

edited

Loading

michelbl Apr 25, 2017

fpagnoux Apr 27, 2017

fpagnoux Apr 27, 2017 •

edited

Loading

michelbl May 3, 2017

benjello May 3, 2017

michelbl May 3, 2017

cbenz Apr 19, 2017

michelbl Apr 25, 2017

cbenz Apr 19, 2017

fpagnoux Apr 27, 2017

benjello Apr 28, 2017

michelbl Apr 28, 2017

fpagnoux commented Apr 27, 2017 •

edited

Loading

fpagnoux commented Apr 27, 2017

		@@ -1,5 +1,13 @@
		# Changelog

		# 17.0.0 - [#???](https://github.com/openfisca/openfisca-france/pull/???)

Remove fuzzy #731

Remove fuzzy #731

Conversation

michelbl commented Apr 14, 2017

fpagnoux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpagnoux Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpagnoux Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fpagnoux commented Apr 27, 2017 • edited Loading

fpagnoux commented Apr 27, 2017

fpagnoux Apr 19, 2017 •

edited

Loading

fpagnoux Apr 27, 2017 •

edited

Loading

fpagnoux commented Apr 27, 2017 •

edited

Loading