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

Relax parameter value warning logic when min or max value is "default" #1578

Merged
merged 2 commits into from
Oct 13, 2017
Merged

Relax parameter value warning logic when min or max value is "default" #1578

merged 2 commits into from
Oct 13, 2017

Conversation

martinholmer
Copy link
Collaborator

This pull request fixes a bug that was revealed in the discussion of PolicyBrain issue 638 beginning with this recent comment. That discussion describes the bug, which arises only when a TaxBrain user implements a reform before 2017 using either the _STD and _STD_Dep parameters.

This Tax-Calculator "bug" is caused by the fact that TaxBrain does not allow users to specify the value of either of these two parameters for widow filing units, and then makes a poor guess about what the TaxBrain user would want the reformed widow value to be. TaxBrain's guessing is poor because it guesses that the user wants to leave the widow value unchanged even though the user is changing the parameter values for other filing unit types. A much better guess would be to set the widow value to the joint value, which is the case in most filing-unit-indexed parameters under current-law. There are only a few cases where the widow value is not equal to the joint value, and in all of those case, the widow value is equal to the single value under current law.

Another pull request could add for the 49 filing-unit-indexed parameters in the current_law_policy.json file a new field that would look like this in most cases:

        "widow_value": "joint_value",

and look like this in the other cases:

        "widow_value": "single_value",

This additional information would allow TaxBrain to make a much better guess than it is now making about what value a TaxBrain user wants for widows.

Of course, the most logical approach would be to add widow boxes to the TaxBrain input GUI so that users could actually specify what parameter value widows should have. And this approach would actually simplify both TaxBrain and Tax-Calculator logic, in comparison to the making-a-better-guess approach outlined above.

@MattHJensen @feenberg @Amy-Xu @andersonfrailey @hdoupe @GoFroggyRun

@codecov-io
Copy link

codecov-io commented Oct 5, 2017

Codecov Report

Merging #1578 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1578   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          37      37           
  Lines        2731    2734    +3     
======================================
+ Hits         2731    2734    +3
Impacted Files Coverage Δ
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 408f82d...12ed745. Read the comment docs.

@martinholmer martinholmer merged commit 27c1b14 into PSLmodels:master Oct 13, 2017
@martinholmer martinholmer deleted the fix-default-warnings branch October 13, 2017 14:37
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.

2 participants