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

Refactor functions in the taxcalc.tbi subpackage and elsewhere #2087

Merged
merged 20 commits into from
Oct 24, 2018
Merged

Refactor functions in the taxcalc.tbi subpackage and elsewhere #2087

merged 20 commits into from
Oct 24, 2018

Conversation

martinholmer
Copy link
Collaborator

@martinholmer martinholmer commented Oct 17, 2018

This pull request refactors functions in the taxcalc/tbi directory so that other models in the Policy Simulation Library (PSL) collection of USA tax models can easily produce the aggregate and distributional tables expected by the TaxBrain web application. It also adds several new tests; the old test results are unchanged. It contains no changes in tax logic or tax results (but see the bottom of this comment).

In the course of this work, the logic of the functions that create the (distribution, difference, aggregate, and diagnostic) results tables has been modified so that the table entries are scaled. Count variables are expressed in millions of filing units and amount variables are expressed in billions of dollars. These scaling changes, which were agreed to in the discussion of #2043, relieves TaxBrain developers of the tedious task of scaling each column of each table in TaxBrain code.

Also, a bug was fixed in commit 425a38f. Ever since the ubi, benefit_cost_total, and benefit_value_total variables were added to the difference table (in PR #1919, which was merged on 2018-03-14, and in PR #1925, which was merged on 2018-03-15), they have been incorrect. Since March they have not been reform-baseline differences (which is what should be in the difference table), but rather reform levels (which are appropriate only for the distribution table). This problem, which has never been reported by any users of Tax-Calculator or TaxBrain, has now been fixed. The fact that no users of Tax-Calculator or TaxBrain reported this bug over a seven month period of time is an interesting observation about how people use the model and what results they look at.

@codecov-io
Copy link

codecov-io commented Oct 17, 2018

Codecov Report

Merging #2087 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2087      +/-   ##
==========================================
+ Coverage   99.91%   99.91%   +<.01%     
==========================================
  Files          16       16              
  Lines        3467     3518      +51     
==========================================
+ Hits         3464     3515      +51     
  Misses          3        3
Impacted Files Coverage Δ
taxcalc/consumption.py 100% <100%> (ø) ⬆️
taxcalc/records.py 100% <100%> (ø) ⬆️
taxcalc/calculator.py 100% <100%> (ø) ⬆️
taxcalc/behavior.py 100% <100%> (ø) ⬆️
taxcalc/growmodel.py 98.18% <100%> (ø) ⬆️
taxcalc/growdiff.py 100% <100%> (ø) ⬆️
taxcalc/__init__.py 100% <100%> (ø) ⬆️
taxcalc/utils.py 100% <100%> (ø) ⬆️
taxcalc/parameters.py 100% <100%> (ø) ⬆️
taxcalc/policy.py 100% <100%> (ø) ⬆️

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 efa6658...aa0bbdb. Read the comment docs.

@martinholmer martinholmer changed the title Refactor functions in the taxcalc.tbi subpackage Refactor functions in the taxcalc.tbi subpackage and elsewhere Oct 19, 2018
@martinholmer martinholmer merged commit ad8fe6a into PSLmodels:master Oct 24, 2018
@martinholmer martinholmer deleted the tbi-reorg branch October 24, 2018 14:43
@MattHJensen
Copy link
Contributor

Thanks for these changes @martinholmer.

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.

3 participants