PSLmodels / Tax-Calculator

USA Federal Individual Income and Payroll Tax Microsimulation Model
https://taxcalc.pslmodels.org
Other
254 stars 153 forks source link

Add documentation on how to analyze TCJA expiration #2734

Closed martinholmer closed 4 months ago

martinholmer commented 4 months ago

This pull request adds only documentation; it includes no substantive changes to the tax calculation logic.

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.42%. Comparing base (2c9ee51) to head (c4cb4f8). Report is 1 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/PSLmodels/Tax-Calculator/pull/2734/graphs/tree.svg?width=650&height=150&src=pr&token=KqtTvRSNjQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels)](https://app.codecov.io/gh/PSLmodels/Tax-Calculator/pull/2734?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) ```diff @@ Coverage Diff @@ ## master #2734 +/- ## ======================================= Coverage 99.42% 99.42% ======================================= Files 13 13 Lines 2594 2594 ======================================= Hits 2579 2579 Misses 15 15 ``` | [Flag](https://app.codecov.io/gh/PSLmodels/Tax-Calculator/pull/2734/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/PSLmodels/Tax-Calculator/pull/2734/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels) | `99.42% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PSLmodels#carryforward-flags-in-the-pull-request-comment) to find out more.
martinholmer commented 4 months ago

@jdebacker asked this question after reviewing PR #2734:

These instructions are very helpful, but to ease the burden on those replicating them, and to provide a concrete example, should their be a full example in the "recipes" using the current policy baseline?

I considered that but felt it was not the best option for three reasons:

martinholmer commented 4 months ago

@jdebacker asked this question in his review of PR #2734:

Should [ext.json] go in the taxcalc/reforms/ directory?

I considered that but rejected it as suboptimal. The main problem is that putting ext.json in the taxcalc/reforms/ directory means that it must be tested by the taxcalc/tests/test_reforms.py script. That would be acceptable if it weren't for the fact that the ext.json reform and its tests results will be (unlike the other reforms) changing several times over the course of the next year. The reasons for these future changes are explained in the tcja_after_2025.md document. My understanding of the files in the taxcalc/reforms/ directory is that they are historical, and therefore, not in flux. So, there would be a considerable extra time burden in maintaining ext.json if it were located in the taxcalc/reforms/ directory.

jdebacker commented 4 months ago

@martinholmer writes:

I considered that but rejected it as suboptimal. The main problem is that putting ext.json in the taxcalc/reforms/ directory means that it must be tested by the taxcalc/tests/test_reforms.py script. That would be acceptable if it weren't for the fact that the ext.json reform and its tests results will be (unlike the other reforms) changing several times over the course of the next year. The reasons for these future changes are explained in the tcja_after_2025.md document. My understanding of the files in the taxcalc/reforms/ directory is that they are historical, and therefore, not in flux. So, there would be a considerable extra time burden in maintaining ext.json if it were located in the taxcalc/reforms/ directory.

Martin, updates to ext.json should themselves be tested. That is good practice. For example, what if a syntax error is introduced in the JSON file? We'd want to be able to catch that. The tests do that.

In addition, the reform directory is the place users are accustom to looking for JSON files like this -- not the docs folder. I could see the case for having an alternative current policy baseline assumption like this in the taxcalc directory (alongside the current law baseline), but I don't see how it makes sense to put this file in the docs/usage directory.

Before merging, I would like to see this added to the reforms folder and tested to help reduce the likelihood errors are introduced in the future. Thank you!

martinholmer commented 4 months ago

@jdebacker said:

Before merging [PR #2734], I would like to see [ext.json] added to the reforms folder and tested to help reduce the likelihood errors are introduced in the future. Thank you!

OK. I have done what you asked.

jdebacker commented 4 months ago

Thank you @martinholmer for this work. Nice addition to allow for a current policy baseline.