Provide simpler syntax to specify policy reforms and assumption updates

[martinholmer]

This pull request resolves issue #2291 by providing Tax-Calculator users with a less complicated syntax with which to specify policy reforms or updates to assumption parameters.

This pull request is the culmination of a series of enhancements to the Parameters class in recent pull requests #2281, #2282, #2283, #2284, #2285, #2289, and #2290. Only this #2292 pull request includes changes that make new versions of Tax-Calculator incompatible with earlier versions of Tax-Calculator. One of the major changes is the one discussed in issue #2291: changing the format of Python dictionaries supplied to the Policy.implement_reform, Consumption.update_consumption, and GrowDiff.update_growdiff methods so that the new param : year : value format is the same as used in JSON reform/assumption files. If you are specifying reforms with dictionaries (rather than in JSON files), you will have to edit the format of the dictionaries so that the old year : param : value format is transformed into the new param : year : value format. See the new Tax-Calculator/new_json.py file for a script that may help you change your personal JSON reform files.

Taken together, all these enhancements to the Parameters class mean that any other model that is already importing the Tax-Calculator package can automatically use this new syntax by creating its own classes derived from the Tax-Calculator Parameters class. An example, of the resulting simplification can be seen in Business-Taxation pull request 88 that specifies a Business-Taxation Policy class derived from the Tax-Calculator Parameters class in just a few lines of code.

This pull request also brings the name of the CPI offset policy parameter into line with other parameter names by changing it from cpi_offset to CPI_offset. A new parameter name-change capability will notify users who forget to use the new name.

The few lines of untested code cannot be tested until this pull request is merged. When it is merged, the @pytest.mark.skip decorators will be removed and those lines of code will be tested.

[codecov]

Codecov Report

Merging #2292 into master will decrease coverage by 0.21%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2292      +/-   ##
==========================================
- Coverage     100%   99.78%   -0.22%     
==========================================
  Files          12       12              
  Lines        2851     2839      -12     
==========================================
- Hits         2851     2833      -18     
- Misses          0        6       +6

Impacted Files	Coverage Δ
taxcalc/calculator.py	99.06% <100%> (-0.94%)	⬇️
taxcalc/consumption.py	100% <100%> (ø)	⬆️
taxcalc/growdiff.py	100% <100%> (ø)	⬆️
taxcalc/parameters.py	100% <100%> (ø)	⬆️
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 ed6413c...257d9d4. Read the comment docs.

[martinholmer]

There has been a substantial discussion of issue #2291, and now that conversation has widened to include the issue of whether or not to make Python reform dictionaries and JSON reform files have the same parameter, year, value order, and if so, which ordering should become the standard.

I agree that this is an important topic, but I think it needs more discussion. Also, this pull request (#2292) is already quite large. So, I propose that #2292 be merged and the additional issue still being discussed can be handled in a follow-up pull request. Does that make sense?

@MattHJensen @andersonfrailey @hdoupe @codykallen @feenberg

[martinholmer]

I said yesterday:

I propose that #2292 be merged and the [parameter-year-value ordering] issue still being discussed can be handled in a follow-up pull request.

Actually, I think we have quickly reached a consensus on the ordering issue, so I'm going to implement the changes to the structure of Python reform/revision dictionaries in this pull request.

[hdoupe]

@martinholmer, what does "vi" stand for?

[martinholmer]

vi stands for vector index.

[hdoupe]

Ah, I see. Thanks.

[hdoupe]

@martinholmer would you mind leaving #2292 open through Monday so that people have a chance to test it out?

[hdoupe]

@martinholmer These names are displayed over the parameter boxes on the webapp:

What do you think about choosing names that are more human readable like:

["single", "married joint", "married separate", "head household", "widow"]

The difference is only a few characters, and they are readable to those who are not familiar with the naming scheme.

[martinholmer]

I think the new values for the vector indexes are more succinct and perfectly understandable to those using Tax-Calculator (or Tax-Brain) via Python programming or the tc command-line tool. If you think users of the TaxBrain webapp need longer labels on the boxes, then by all means you should supply them with longer box labels. That's your call.

[martinholmer]

@hdoupe asked:

would you mind leaving #2292 open through Monday so that people have a chance to test it out?

OK.

[hdoupe]

@martinholmer Policy.implement_reform does not accept years that are strings:

import taxcalc
import json

# 2.0 styled reform
ref = {
    "policy": {
        "STD": {"2018": [10001, 10000, 10000, 10000, 10000]},
        "STD-indexed": {"2018": False},
    }
}
ref_json = json.dumps(ref)

# Use read_json_param_objects to parse JSON reform
pol = taxcalc.Calculator.read_json_param_objects(ref_json, None)

# Note that keys are integers now.
print("parsed params: ", pol["policy"])

print("Parsed parameters load without errors.")
taxcalc.Policy().implement_reform(pol["policy"])

print("Parameters with string years throw an error.")
taxcalc.Policy().implement_reform(ref["policy"])

outputs:

(taxcalc-dev) HDoupe-MacBook-Pro:Tax-Calculator henrydoupe$ python test.py 
parsed params:  {'STD': {2018: [10001, 10000, 10000, 10000, 10000]}, 'STD-indexed': {2018: False}}
Parsed parameters load without errors.
Parameters with string years throw an error.
Traceback (most recent call last):
  File "test.py", line 23, in <module>
    taxcalc.Policy().implement_reform(ref["policy"])
  File "/Users/henrydoupe/Documents/Tax-Calculator/taxcalc/policy.py", line 108, in implement_reform
    self._update(reform, print_warnings, raise_errors)
  File "/Users/henrydoupe/Documents/Tax-Calculator/taxcalc/parameters.py", line 292, in _update
    raise ValueError(msg.format(name, year))
ValueError: ERROR: KEY STD YEAR 2018 is not an integer year
(taxcalc-dev) HDoupe-MacBook-Pro:Tax-Calculator henrydoupe$ 

It seems like implement_reform should accept years that are integers or strings. Users may not understand why the JSON files require the years to be strings, but the implement_reform method requires them to be integers. This would also make it easier to serialize and deserialize reform parameters that are accepted by implement_reform.

[martinholmer]

@hdoupe reported in PR #2292 that:

Policy.implement_reform does not accept years that are strings

Yes, and that is by design. Thanks for calling attention to the need to improve various method docstrings to clarify how things work. I've improved the docstrings in commit 75deaa3.

[hdoupe]

Ok, I see. Glad my question was helpful.