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

Add TCJA preset #792

Merged
merged 9 commits into from Jan 9, 2018
Merged

Add TCJA preset #792

merged 9 commits into from Jan 9, 2018

Conversation

hdoupe
Copy link
Collaborator

@hdoupe hdoupe commented Dec 23, 2017

This PR adds the TCJA preset. Due to the number of parameters involved in creating this preset, I went through considerable lengths to make sure I put everything together correctly. I included the preset creation and testing scripts so that others can check my work.

  1. I manually translated the TCJA_Reconciliation.json file to a dictionary that can be programmatically posted to TaxBrain. I programmatically posted this file to the test app. I then compared the results from this method with the results from uploading TCJA_Reconciliation.json via the file input interface.
  2. I used TaxBrain GUI input parsing functions to convert this dictionary to a Tax-Calculator styled dictionary and the TCJA_Reconciliation.json file into a Tax-Calculator styled dictionary. I then compared parameters and parameter values to make sure that the file and the dictionary are equivalent.

You can check the file post here: http://ospc-taxes7.herokuapp.com/taxbrain/1507/
You can check the programmatic post here: http://ospc-taxes7.herokuapp.com/taxbrain/1510/
[EDIT] Production post is here: https://www.ospc.org/taxbrain/edit/30126/?start_year=2018

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 4, 2018

PR #792 passes with changes from #790. See:

(aei_dropq) HDoupe-MacBook-Pro:PolicyBrain henrydoupe$ git pull origin reverse_character
From https://github.com/hdoupe/PolicyBrain
 * branch            reverse_character -> FETCH_HEAD
Auto-merging templates/taxbrain/input_form.html
Merge made by the 'recursive' strategy.
 templates/taxbrain/input_form.html         |  1 +
 webapp/apps/taxbrain/forms.py              |  5 +++--
 webapp/apps/taxbrain/helpers.py            | 36 +++++++++++++++++++++++++++++++++---
 webapp/apps/taxbrain/models.py             |  2 +-
 webapp/apps/taxbrain/tests/test_helpers.py | 15 ++++++++++++---
 webapp/apps/taxbrain/tests/test_views.py   | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 webapp/apps/taxbrain/views.py              |  3 ++-
 webapp/apps/test_assets/test_reform.py     | 11 ++++++++---
 8 files changed, 114 insertions(+), 16 deletions(-)
(aei_dropq) HDoupe-MacBook-Pro:PolicyBrain henrydoupe$ py.test webapp/apps/test_assets/test_tcja.py -s -v
======================================================= test session starts =======================================================
platform darwin -- Python 2.7.14, pytest-3.3.0, py-1.5.2, pluggy-0.6.0 -- /anaconda/envs/aei_dropq/bin/python
cachedir: .cache
Django settings: webapp.settings (from ini file)
rootdir: /Users/henrydoupe/Documents/PolicyBrain, inifile: pytest.ini
plugins: django-3.1.2, hypothesis-3.38.5, celery-4.1.0
collected 1 item                                                                                                                  

webapp/apps/test_assets/test_tcja.py::test_tcja_construction _AMT_em
_ID_AllTaxes_c
_STD
_CTC_ps
_PT_brk6
_PT_brk5
_PT_brk4
_PT_brk3
_PT_brk2
_PT_brk1
_AMT_em_ps
_II_brk6
_II_brk5
_II_brk4
_II_brk1
_II_brk3
_II_brk2
_AMT_em
_ID_AllTaxes_c
_STD
_CTC_ps
_PT_brk6
_PT_brk5
_PT_brk4
_PT_brk3
_PT_brk2
_PT_brk1
_AMT_em_ps
_II_brk6
_II_brk5
_II_brk4
_II_brk1
_II_brk3
_II_brk2
PASSED                                                         [100%]

==================================================== 1 passed in 5.18 seconds =====================================================

@hdoupe hdoupe added the ready label Jan 4, 2018
This was referenced Jan 4, 2018
@GoFroggyRun
Copy link
Contributor

The reform conversion looks pretty cool. I'm not sure, however, whether we want to have a preset for TCJA reconciliation now that tax-calculator is working on making TCJA reconciliation the current law (see #1803). Once that is done (which I think will happen very soon), wouldn't we have to remove this preset after all? On the other hand, a preset link for the current current law would seem more helpful after #1803 is merged into tax-calculator.

@hdoupe @MattHJensen what's your thoughts on this?

@martinholmer
Copy link
Contributor

@hdoupe, You need to work against the TCJA_Reconciliation.json file in Tax-Calculator pull request 1803, which is a corrected version of the one on the master branch as discussed in 1803.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 5, 2018

Thanks @GoFroggyRun.

I'm not sure, however, whether we want to have a preset for TCJA reconciliation now that tax-calculator is working on making TCJA reconciliation the current law (see #1803).

I saw that. When I put this together, I was under the impression that there would be a delay before the current_law_policy.json file was ready. Further, I was responding to the request in #787.

Once that is done (which I think will happen very soon), wouldn't we have to remove this preset after all? On the other hand, a preset link for the current current law would seem more helpful after #1803 is merged into tax-calculator.

I agree with this.

@martinholmer Thanks for pointing this out. The only change that I need to make is the _ID_Medical_frt parameter, correct?

@martinholmer
Copy link
Contributor

@hdoupe asked about corrections to TCJA_Reconciliation.json file:

Thanks for pointing this out. The only change that I need to make is the _ID_Medical_frt parameter, correct?

Right. The start year is actually 2017 (not 2018). That's the only change.

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 6, 2018

@martinholmer said:

Right. The start year is actually 2017 (not 2018). That's the only change.

Do you mean that the reform start year is 2017? Or, just the start year for _ID_Medical_frt is 2017?

@martinholmer
Copy link
Contributor

@hdoupe asked:

Do you mean that the reform start year is 2017? Or, just the start year for _ID_Medical_frt is 2017?

From Tax-Calculator pull request 1803 we have this change in the taxcalc/reforms/TCJA_Reconciliation.json file:

        "_ID_Medical_frt":
 -            {"2018": [0.075],
 +            {"2017": [0.075],
               "2019": [0.1]},

@hdoupe
Copy link
Collaborator Author

hdoupe commented Jan 6, 2018

@martinholmer Thanks for clarifying. Please, let me know if this looks better.

@martinholmer
Copy link
Contributor

@hdoupe said:

Please, let me know if this looks better.

Seems correct.

@hdoupe hdoupe merged commit 1ebf21b into ospc-org:master Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants