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

Modify pass-through exclusion calculation #1819

Merged
merged 4 commits into from
Jan 11, 2018

Conversation

codykallen
Copy link
Contributor

This PR modifies the calculation of the business income exclusion for the TCJA. This renames two parameters: PT_exclusion_rt becomes PT_excl_rt, and PT_exclusion_wage_limit becomes PT_excl_wagelim_rt. This PR also adds two parameters, PT_excl_wagelim_thd and PT_excl_wagelim_prt, which are used to correctly implement the rules for the wage limitation on the business income exclusion in the final version of the TCJA. This PR also changes the names of the parameters in the TCJA json files and adds the two new parameters to the TCJA_Reconciliation.json (although I did not correct the TCJA Senate versions).

The default values for the two new parameters are set to maintain backward compatibility, both with pre-TCJA law and with the TCJA Senate json files.

This PR fulfills the suggestions in #1816.

@martinholmer
Copy link
Collaborator

@codykallen, thanks for PR #1819.
That was quick, probably took you less time than helping me do it.
I'll begin a review now.

@codecov-io
Copy link

codecov-io commented Jan 10, 2018

Codecov Report

Merging #1819 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1819   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          37      37           
  Lines        3061    3058    -3     
======================================
- Hits         3061    3058    -3
Impacted Files Coverage Δ
taxcalc/__init__.py 100% <0%> (ø) ⬆️
taxcalc/utils.py 100% <0%> (ø) ⬆️

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 9843edf...27f4973. Read the comment docs.

@martinholmer
Copy link
Collaborator

martinholmer commented Jan 10, 2018

@codykallen, one thing would help. I know the tests are telling you to update the docs/index.html file (because you revised the contents of the current_law_policy.json file), but that's not the best thing to do. I'm still trying to figure out how to handle this better, maybe by making the test that checks whether or not the docs/index.html file is up-to-date be marked pre_release.

The problem with the test now is that if you follow its directions (which you did) then, immediately after this pull request is merged, the documentation will be updated even though there is no taxcalc package available with the new PT limitation parameters.

The way to "fix" this for now is on your modify_pt_exclusion branch run these git commands:

git checkout -- docs/index.html
git push origin modify_pt_exclusion

Sorry about the hassle.

Now I'll start my review of #1819.

@codykallen
Copy link
Contributor Author

@martinholmer, I just ran the two commands you suggested, and they don't appear to have done anything. Do you have a different suggestion?

@martinholmer
Copy link
Collaborator

@codykallen said:

I just ran the two commands you suggested, and they don't appear to have done anything.
Do you have a different suggestion?

Right. I forgot you've already committed the new docs/index.html file.
Just let things be and I'll try to fix this (which is my fault, after all) later.

@martinholmer
Copy link
Collaborator

martinholmer commented Jan 10, 2018

@codykallen, Everything in #1819 so far looks good to me.

BUT you said in issue #1816 this:

There is actually another related piece that needs to be modified, although it would apply separately (I believe in the CapGains function or the AGI function). The TCJA also limits pass-through losses (e00900 + e26270) to $500,000 for married filers and $250,000 for other filers, although I doubt this provision will affect many filers.

Do you want to include this enhancement in pull request #1819?

Does this enhancement require any new policy parameters or is it just a change in logic that uses existing policy parameters?

@codykallen
Copy link
Contributor Author

@martinholmer, this enhancement would require a one new policy parameter, as well as changing the TCJA_Reconciliation.json file. However, the logic would be relatively straightforward, if implemented carefully. Do you want me to add this as well?

@martinholmer
Copy link
Collaborator

@codykallen said:

this [second] enhancement [mentioned in #1816] would require one new policy parameter, as well as changing the TCJA_Reconciliation.json file. However, the logic would be relatively straightforward, if implemented carefully. Do you want me to add this as well?

Yes, that would be great! Just add one or more commits to your modify_pt_exclusion branch.

@codykallen
Copy link
Contributor Author

@martinholmer, this adds the parameter an an above-the-line deduction cap.

@martinholmer martinholmer added ready and removed ready labels Jan 10, 2018
@martinholmer
Copy link
Collaborator

martinholmer commented Jan 10, 2018

@codykallen, Thanks for adding commit 75d1439 that represents the business loss limitation policy parameter and logic under TCJA.

It all looks good except for one thing. If TCJA limits business losses in the following code:

    # compute ymod1 variable that is included in AGI
    ymod1 = (e00200 + e00700 + e00800 + e01400 + e01700 +
             invinc - invinc_agi_ec + e02100 + e02300 +
             max(e00900 + e02000, -ALD_BusinessLosses_c[MARS - 1]))

and ymod1 is added into AGI, we are going to get a different expanded_income for those with limited business losses. I don't think we want a policy reform changing people's expanded_income.

There is an easy way to fix this, I think. We already have a variable called c02900_in_ei, which keeps track of the income that is included in expanded_income but is excluded from AGI. I think you should modify the CapGains function so that c02900_in_ei is a function argument and is returned by the CapGains function. And then inside the CapGains function compute the amount of the business loss limitation (as a non-positive number) and then add that limitation to the c02900_in_ei variable.

Does this make sense?

@codykallen
Copy link
Contributor Author

@martinholmer, I believe this should be ready to go now.

@martinholmer martinholmer added ready and removed WIP labels Jan 11, 2018
@martinholmer
Copy link
Collaborator

@codykallen, thanks for commit 9f4cd03, but there seems to be one thing missing.
You need to add c02900_in_ei to the tuple of variables returned from the CapGains function.

@martinholmer
Copy link
Collaborator

@codykallen, Thanks for all the help on these TCJA enhancements!

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