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

Make altersentlastungsbetrag dependent on age not on current date #385

Merged
merged 8 commits into from May 17, 2022

Conversation

m-pannier
Copy link
Collaborator

@m-pannier m-pannier commented May 4, 2022

What problem do you want to solve?

Closes #350

This pull request makes the parameters altersentlastungsbetrag_max and altersentlastung_quote dependent on age and thus corrects the current modeling problems as specified in #350. We use geburtsjahr to control for age (see here for an example table) and basically apply the solution that is currently used for the parameter behinderten_pauschbetrag.

We introduce two different functions eink_st_altersfreib_bis_2004 and eink_st_altersfreib_ab_2005 to take account of the policy change due to the Alterseinkünftegesetz which replace the single function eink_st_altersfreib.

We assign individuals who were born prior to 1940 the values that apply to individuals born in 1940.

Todo

  • If it is a relevant contribution, put it in CHANGES.rst.

Copy link
Member

@ChristianZimpelmann ChristianZimpelmann left a comment

Choose a reason for hiding this comment

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

Great job! Thank you very much! I added two comments. Additional remarks:

  • Please add your contribution to CHANGES.rst
  • The tests are not running through currently. Please have another look at that and make sure that they run through on your machine (let me know if you cannot fix the errors).
  • It would be great if we had a few test cases that directly test eink_st_altersfreib. Probably in test_zu_verst_eink.py. If I understand correctly, the changes you made should change taxable income for some subjects. (or maybe there are tests and that is why the tests are failing?)

gettsim/parameters/eink_st_abzuege.yaml Outdated Show resolved Hide resolved
gettsim/parameters/eink_st_abzuege.yaml Outdated Show resolved Hide resolved
@lillyfischer
Copy link
Collaborator

I integrated your comments @ChristianZimpelmann!

Remaining issue:
As you said, the tests fail, in particular test_zu_verst_eink.py fails.
test_zu_verst_eink.py already contains some tests on eink_st_altersfreib.
I added the variable geburtstjahr to the inputs as this is needed for the new function eink_st_freibetrag_ab2005.

Unfortunately, I don't understand sufficiently how these tests work. Maybe we could do a little zoom session, where you explain this to me and @m-pannier ?

Copy link
Member

@ChristianZimpelmann ChristianZimpelmann left a comment

Choose a reason for hiding this comment

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

@m-pannier @lillyfischer,

As @LauraGergeleit is currently rewriting the test_zu_verst_eink test cases and might need your changes for it, I propose the following:

  • We merge this PR with test_zu_verst_eink set to be ignored by pytask (I just pushed a commit that does this).
  • After Laura is done, you add some test cases within Fix rtol in tests and associated bugs #392 that test the changes you just implemented here.

Is there anything else that is still open to do in this PR from your side? Otherwise, we should be ready to merge once the tests have been run through.

It looks really good! Thank you for the great work!

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #385 (1e7445f) into main (dd963df) will decrease coverage by 0.99%.
The diff coverage is 95.45%.

@@            Coverage Diff             @@
##             main     #385      +/-   ##
==========================================
- Coverage   92.10%   91.10%   -1.00%     
==========================================
  Files          76       76              
  Lines        3520     3541      +21     
==========================================
- Hits         3242     3226      -16     
- Misses        278      315      +37     
Impacted Files Coverage Δ
gettsim/taxes/zu_verst_eink/freibetraege.py 71.42% <93.75%> (-28.58%) ⬇️
gettsim/policy_environment.py 100.00% <100.00%> (ø)
gettsim/tests/test_zu_verst_eink.py 47.22% <100.00%> (-49.93%) ⬇️
gettsim/taxes/zu_verst_eink/eink.py 88.88% <0.00%> (-11.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd963df...1e7445f. Read the comment docs.

@ChristianZimpelmann ChristianZimpelmann merged commit 320bab4 into main May 17, 2022
@ChristianZimpelmann ChristianZimpelmann deleted the correct-altersentlastung branch May 17, 2022 07:05
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.

Make Altersentlastungsbetrag depending on age not on current date
3 participants