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

Add logic to report reform parameter warnings and errors #1524

Merged
merged 9 commits into from
Aug 24, 2017
Merged

Add logic to report reform parameter warnings and errors #1524

merged 9 commits into from
Aug 24, 2017

Conversation

martinholmer
Copy link
Collaborator

@martinholmer martinholmer commented Aug 22, 2017 •
edited
Loading

This pull request implements the approach to warnings and errors discussed at the end of the discussion in TaxBrain issue 630. As such, it replaces the less flexible approach used in #1502.

The Tax-Calculator CLI, tc, now uses these warning and error messages. See the changes to the taxcalcio.py file for details.

TaxBrain can use this new Tax-Calculator capability by calling the reform_warnings_errors function in the dropq.py file, which is new to the Tax-Calculator public API. Read this new function's docstring carefully before using it in TaxBrain. The consensus in #630 is to show warnings and proceed with tax calculation if there are no errors. If there are errors, we should show all the warning and error messages and then stop without doing any tax calculations. That is what the Tax-Calculator CLI, tc, does now.

If anyone sees a better way to provide these capabilities, please make your suggestion here.

@MattHJensen @feenberg @Amy-Xu @andersonfrailey @hdoupe @GoFroggyRun @codykallen
@jdebacker @rickecon @brittainhard @talumbau

@codecov-io
Copy link

codecov-io commented Aug 22, 2017 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1524   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          37      37           
  Lines        2521    2565   +44     
======================================
+ Hits         2521    2565   +44
Impacted Files Coverage Δ
taxcalc/policy.py 100% <100%> (ø) ⬆️
taxcalc/taxcalcio.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 ac938b2...71d9b80. Read the comment docs.

@martinholmer
Copy link
Collaborator Author

martinholmer commented Aug 22, 2017 •
edited
Loading

Using the CLI tc built using the code in this pull request, here is how tc now handles warnings and errors:

$ cat bad_reform.json 
// bad_reform.json contains a logically incorrect attempt to completely
// eliminate the income tax (leaving refundable credits unchanged)
{"policy": {
    "_II_rt1": {"2020": [0.0]},
    "_II_brk1": {"2020": [[9e99, 9e99, 9e99, 9e99, 9e99]]},
    "_CG_rt1": {"2020": [0.0]},
    "_CG_rt2": {"2020": [0.0]},
    "_CG_rt3": {"2020": [0.0]},
    "_CG_rt4": {"2020": [0.0]},
    "_AMT_rt1": {"2020": [0.0]},
    "_AMT_rt2": {"2020": [0.0]},
    "_AMT_CG_rt1": {"2020": [0.0]},
    "_AMT_CG_rt2": {"2020": [0.0]},
    "_AMT_CG_rt2": {"2020": [0.0]},
    "_AMT_CG_rt3": {"2020": [0.0]},
    "_AMT_CG_rt4": {"2020": [0.0]}
}}

$ cat warn_reform.json 
// warn_reform.json contains parameter value implying "data weakness" warning
{"policy": {"_STD_Dep": {"2020": [0.0]}}}

$ ls puf-20-*
ls: puf-20-*: No such file or directory

$ tc puf.csv 2020 --reform bad_reform.json
ERROR: 2020 _II_brk1 value 9e+99 > max value 40605.4 for _II_brk2
ERROR: 2020 _II_brk1 value 9e+99 > max value 81210.81 for _II_brk2
ERROR: 2020 _II_brk1 value 9e+99 > max value 40605.4 for _II_brk2
ERROR: 2020 _II_brk1 value 9e+99 > max value 54354.53 for _II_brk2
ERROR: 2020 _II_brk1 value 9e+99 > max value 81210.81 for _II_brk2
ERROR: 2021 _II_brk1 value 9e+99 > max value 41551.51 for _II_brk2
ERROR: 2021 _II_brk1 value 9e+99 > max value 83103.02 for _II_brk2
ERROR: 2021 _II_brk1 value 9e+99 > max value 41551.51 for _II_brk2
ERROR: 2021 _II_brk1 value 9e+99 > max value 55620.99 for _II_brk2
ERROR: 2021 _II_brk1 value 9e+99 > max value 83103.02 for _II_brk2
ERROR: 2022 _II_brk1 value 9e+99 > max value 42515.51 for _II_brk2
ERROR: 2022 _II_brk1 value 9e+99 > max value 85031.01 for _II_brk2
ERROR: 2022 _II_brk1 value 9e+99 > max value 42515.51 for _II_brk2
ERROR: 2022 _II_brk1 value 9e+99 > max value 56911.4 for _II_brk2
ERROR: 2022 _II_brk1 value 9e+99 > max value 85031.01 for _II_brk2
ERROR: 2023 _II_brk1 value 9e+99 > max value 43506.12 for _II_brk2
ERROR: 2023 _II_brk1 value 9e+99 > max value 87012.23 for _II_brk2
ERROR: 2023 _II_brk1 value 9e+99 > max value 43506.12 for _II_brk2
ERROR: 2023 _II_brk1 value 9e+99 > max value 58237.44 for _II_brk2
ERROR: 2023 _II_brk1 value 9e+99 > max value 87012.23 for _II_brk2
ERROR: 2024 _II_brk1 value 9e+99 > max value 44519.81 for _II_brk2
ERROR: 2024 _II_brk1 value 9e+99 > max value 89039.61 for _II_brk2
ERROR: 2024 _II_brk1 value 9e+99 > max value 44519.81 for _II_brk2
ERROR: 2024 _II_brk1 value 9e+99 > max value 59594.37 for _II_brk2
ERROR: 2024 _II_brk1 value 9e+99 > max value 89039.61 for _II_brk2
ERROR: 2025 _II_brk1 value 9e+99 > max value 45561.57 for _II_brk2
ERROR: 2025 _II_brk1 value 9e+99 > max value 91123.14 for _II_brk2
ERROR: 2025 _II_brk1 value 9e+99 > max value 45561.57 for _II_brk2
ERROR: 2025 _II_brk1 value 9e+99 > max value 60988.88 for _II_brk2
ERROR: 2025 _II_brk1 value 9e+99 > max value 91123.14 for _II_brk2
ERROR: 2026 _II_brk1 value 9e+99 > max value 46632.27 for _II_brk2
ERROR: 2026 _II_brk1 value 9e+99 > max value 93264.53 for _II_brk2
ERROR: 2026 _II_brk1 value 9e+99 > max value 46632.27 for _II_brk2
ERROR: 2026 _II_brk1 value 9e+99 > max value 62422.12 for _II_brk2
ERROR: 2026 _II_brk1 value 9e+99 > max value 93264.53 for _II_brk2
USAGE: tc --help

$ ls puf-20-*
ls: puf-20-*: No such file or directory

$ tc puf.csv 2020 --reform warn_reform.json
You loaded data for 2009.
Tax-Calculator startup automatically extrapolated your data to 2020.
PARAMETER VALUE WARNING(S):   (read documentation)
WARNING: 2020 _STD_Dep value 0.0 < min value 1123.47
WARNING: 2021 _STD_Dep value 0.0 < min value 1149.65
WARNING: 2022 _STD_Dep value 0.0 < min value 1176.32
WARNING: 2023 _STD_Dep value 0.0 < min value 1203.73
WARNING: 2024 _STD_Dep value 0.0 < min value 1231.78
WARNING: 2025 _STD_Dep value 0.0 < min value 1260.6
WARNING: 2026 _STD_Dep value 0.0 < min value 1290.22
CONTINUING WITH CALCULATIONS...

$ ls puf-20-*
puf-20-warn_reform-#.csv

Notice that there is no output from the reform that contains a logic error, but that tc continued to execute after writing out just warnings and produced the results in the puf-20-warn_reform-#.csv file.

@martinholmer martinholmer reopened this Aug 23, 2017
@martinholmer
Copy link
Collaborator Author

Unless I hear any suggestions about how to better implement reform parameter warnings and errors, I plan to merge #1524 at the end of the work day on Thursday, August 24.

@MattHJensen @feenberg @Amy-Xu @andersonfrailey @hdoupe @GoFroggyRun @codykallen
@jdebacker @rickecon @brittainhard @talumbau

@martinholmer martinholmer mentioned this pull request Aug 24, 2017
Merged
@martinholmer martinholmer merged commit caf653e into PSLmodels:master Aug 24, 2017
@martinholmer martinholmer deleted the warnings-vs-errors branch August 24, 2017 21:36
@MattHJensen MattHJensen mentioned this pull request Aug 26, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martinholmer @codecov-io @talumbau