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.

Sign up for GitHub

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

Jump to bottom

TCJA reconciliation reform file #1759

Merged
merged 5 commits into from
Dec 16, 2017
Merged

TCJA reconciliation reform file #1759

merged 5 commits into from
Dec 16, 2017

Conversation

codykallen
Copy link
Contributor

This PR adds a reform file for the reconciliation version of the TCJA.

@codecov-io
Copy link

codecov-io commented Dec 15, 2017 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1759   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          37      37           
  Lines        2984    3000   +16     
======================================
+ Hits         2984    3000   +16
Impacted Files Coverage Δ
taxcalc/behavior.py 100% <0%> (ø) ⬆️
taxcalc/records.py 100% <0%> (ø) ⬆️
taxcalc/calculate.py 100% <0%> (ø) ⬆️
taxcalc/taxcalcio.py 100% <0%> (ø) ⬆️
taxcalc/macro_elasticity.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 9ba0053...c557124. Read the comment docs.

MaxGhenis
MaxGhenis reviewed Dec 15, 2017
View reviewed changes
Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

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

typos

taxcalc/reforms/TCJA_Reconciliation.json Outdated
@@ -0,0 +1,174 @@
// Title: Tax Cuts and Jobs Act, Reconciliation verson
Copy link
Contributor

Choose a reason for hiding this comment

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

version

taxcalc/reforms/TCJA_Reconciliation.json Outdated
// - Modification of Alternative Minimum Tax exemption (6)
// - Repeal of certain above the line deductions (7)
// - Changes to itemized deductions (8)
// - Switch to chained CPI from CPI-U for tax paramter adjustment (9)
Copy link
Contributor

Choose a reason for hiding this comment

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

parameter

@martinholmer
Copy link
Collaborator

@codykallen, We all appreciate your fast, high-quality work, but this time don't forget to run all the tests before you merge this PR.

In #1756, you forgot to update the user documentation (which was done in #1757).

Just run all the tests locally before your final commit to this pull request.

MattHJensen
MattHJensen reviewed Dec 15, 2017
View reviewed changes
taxcalc/reforms/TCJA_Reconciliation.json
{"2018": [[38700, 77400, 38700, 51800, 77400]],
"2026": [[45751, 91502, 45751, 61242, 91502]]},
"_II_brk3":
{"2018": [[82500, 165000, 82500, 82500, 165000]],
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't single be 70000?

Copy link
Contributor

Choose a reason for hiding this comment

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

(and separate?)

Copy link
Contributor Author

@codykallen codykallen Dec 15, 2017 •
edited by MattHJensen
Loading

Choose a reason for hiding this comment

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

No. The correct number, at least according to the text of the bill and the section-by-section summary, is $82,500.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Thanks.

MattHJensen
MattHJensen reviewed Dec 15, 2017
View reviewed changes
taxcalc/reforms/TCJA_Reconciliation.json
{"2018": [[82500, 165000, 82500, 82500, 165000]],
"2026": [[110791, 184571, 92286, 158169, 184571]]},
"_II_brk4":
{"2018": [[157500, 315000, 157500, 157500, 315000]],
Copy link
Contributor

Choose a reason for hiding this comment

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

Single/Separate -> 160000

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the bill specifies $157,500. See the link in the JSON, page 3 of the legislative text.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

157,500 seems to be right according to http://docs.house.gov/billsthisweek/20171218/CRPT-115HRPT-466.pdf, page 2 and 3.

@MattHJensen
Copy link
Contributor

MattHJensen commented Dec 16, 2017 •
edited
Loading

pg 151 on the AMT:

The phaseout thresholds are increased to $1,000,000 for married taxpayers filing a joint return, and $500,000 for all other taxpayers (other than estates and trusts). These amounts are indexed for inflation.

@MattHJensen
Copy link
Contributor

This looks good to me other than my one outstanding comment.

@evtedeschi3, @GoFroggyRun, what do you think?

@GoFroggyRun
Copy link
Contributor

I've checked all 2026 values, and they look good to me. +1

@ernietedeschi
Copy link
Contributor

ernietedeschi commented Dec 16, 2017 via email

@MattHJensen
Copy link
Contributor

@codykallen, are this and this good?

@GoFroggyRun
Copy link
Contributor

GoFroggyRun commented Dec 16, 2017 •
edited
Loading

Seems like [500000, 1000000, 500000, 500000, 1000000] are the right values for _AMT_em_ps in 2018.

@codykallen
Copy link
Contributor Author

@martinholmer, running docs/make_index.py does not appear to have changed any files.

@codykallen
Copy link
Contributor Author

@MattHJensen, trying to update docs/index.html doesn't seem to be doing anything. Do you have a specific recommendation?

@MattHJensen
Copy link
Contributor

MattHJensen commented Dec 16, 2017 •
edited
Loading

@MattHJensen, trying to update docs/index.html doesn't seem to be doing anything. Do you have a specific recommendation?

I believe that means docs/index.html doesn't need to be updated. I am going to merge this so that @martinholmer can respond to your question on a more leisurely timeline.

Thanks very much to @codykallen for contributing this and to @evtedeschi3 and @GoFroggyRun for your review. I expect that some of our users will be working with this tonight, and I will follow up if they have any additional feedback.

@MattHJensen MattHJensen merged commit 49dd7a6 into PSLmodels:master Dec 16, 2017
@codykallen codykallen deleted the tcja_reconciliation branch December 16, 2017 00:55
@martinholmer
Copy link
Collaborator

@codykallen, Adding only a reform file did not trigger a change in user documentation, but adding the new reform file did require a change in the taxcalc/reforms/REFORMS.md file (which has been taken care of in PR#1760). Thanks for all the work involved in quickly developing the JSON reform file for the conference committee bill.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxGhenis MaxGhenis MaxGhenis left review comments

@MattHJensen MattHJensen MattHJensen left review comments

@GoFroggyRun GoFroggyRun GoFroggyRun left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@codykallen @codecov-io @martinholmer @MattHJensen @GoFroggyRun @ernietedeschi @MaxGhenis