openfisca / legislation-explorer

Explore legislation formulas and parameters.
https://legislation.demo.openfisca.org
GNU Affero General Public License v3.0
26 stars 12 forks source link

Render scales #75

Closed cbenz closed 7 years ago

cbenz commented 7 years ago

Connected to #73 Fixes #74

The fix of #74 is needed for this PR, it is not a "two-features" PR ;-)

cbenz commented 7 years ago

@fpagnoux @MattiSG I'm not sure about the best way to test this PR with Watai. I'm open to propositions.

fpagnoux commented 7 years ago

Displays:

image

fpagnoux commented 7 years ago

I find it difficult to distinguish the different groups.

Suggestion: make the bottom border thicker and darker like this:

image

fpagnoux commented 7 years ago

I'm not sure about the best way to test this PR with Watai

What is the difficulty you are encountering ? Testing the table ?

cbenz commented 7 years ago

@fpagnoux I'd like to have some suggestions about which new Watai test to write.

fpagnoux commented 7 years ago

🚨 FORCE PUSH DETECTED DURING REWIEW🚨 👮 🚔

cbenz commented 7 years ago

New rendering after my new commit:

image

benjello commented 7 years ago

I don't know if it is the right place to make the following remarks but I put the here anyway:

fpagnoux commented 7 years ago

Some afterthoughts:

Date Tranche Taux marginal
fpagnoux commented 7 years ago

About testing, following the spirit of the current tests, I'd have the following scenario :

If there is in france a case of a marginal scale that doesn't exist anymore (@benjello ?) , I'd also test its first line.

I would not test the values of the row span, rates and threshold, as this would make our tests break at any parameter update. This is a hole in our test net, but the maintenance cost is too high (except if one day we decide to run the tests on a dummy country).

fpagnoux commented 7 years ago
  • Rate should represented as percentage values i.e 5,5 %

So far, we are not handling units in the new version of the API and therefore in the legislation explorer. This may come later.

cbenz commented 7 years ago

So far, we are not handling units in the new version of the API and therefore in the legislation explorer.

@fpagnoux But if you suggest adding a header with "Taux marginaux", why not adding a % unit?

cbenz commented 7 years ago

@fpagnoux @benjello Is this rendering OK for you (I took it from a random website)?

image

For the first bracket, if we say Entre 0.0 et 9700.0, I think 9700 is included.

But for the following brackets, would it be Entre 9701 et 26791?

Is it worth doing the + 1 operation?


Given this data from the API:

{
    "0.0": 0.0,
    "9700.0": 0.14,
    "26791.0": 0.3,
    "71826.0": 0.41,
    "152108.0": 0.45
}

I didn't see it explained explicitly in https://github.com/openfisca/openfisca-web-api/issues/107 so I wanted to make it clear.

cbenz commented 7 years ago

My last commit renders like that:

image

Is is OK?

fpagnoux commented 7 years ago

I would not add the +1, I find it misleading.

Conceptually you pay 0% for what is under 9700 and 14% for what is above 9700.

9701 only plays a role because the income to tax happens to be rounded before applying the scale. Without rounding, which may be the case for some scales, it doesn't make sense: with 9700.5, I'm already in the 2nd bracket.

Also, when I read Entre 9701 et..., I understand that my 9701th euro won't be taxed, which is not the case.

fpagnoux commented 7 years ago

So far, we are not handling units in the new version of the API and therefore in the legislation >explorer.

@fpagnoux But if you suggest adding a header with "Taux marginaux", why not adding a % unit?

You're right. As we are in the case of a marginal scale, we know the unit is percent, so 👍 for me.