add Behavior class and move defaukt behavioral assumptions to behavior.json

[MattHJensen]

This PR places behavioral assumptions, such as income and substitution effects, in params.json and uses them in behavior().

This is probably not a good long-term solution because everything else about our Parameters class has to do with "tax law parameters" rather than "economic parameters." In fact, I suggested a component of a different solution on Feb 21 in #138. @martinholmer, @talumbau and I have also very briefly discussed other options on a phone call at some point.

That said, @PeterDSteinberg and I want to place these economic assumptions in params.json for now while we think of a better long-term home because our Parameters class provides all the functionality necessary to make these economic assumptions available to webapp users.

@martinholmer, I hope you have a chance to look at this.

[martinholmer]

I agree with @MattHJensen so strongly that I think it is a very bad idea to commit this pull request. Why not put the behavioral assumptions in their own JSON file? The default values could be put in a file named behavior.json. Then add to behavior.py a function read_behavioral_assumptions(filename="behavior.json") that reads a specified json file containing behavioral assumptions and returns an assumptions dictionary? Then the Calculator class constructor would need another parameter: the behavioral assumption dictionary returned by the read_behavioral_assumptions() function. Mixing these behavioral parameters in with the tax policy parameters is asking for nothing but trouble.

[MattHJensen]

@martinholmer many thanks for looking this over. We are in agreement about the risks of mixing the behavioral and tax policy parameters, although as you can tell I was tempted to accept those in the short run in order to get behavior up and running on the webapp. @PeterDSteinberg and I were just discussing some other options, though, that might still be easy to integrate with the webapp but wouldn't pollute params.json or the Parameters object. @PeterDSteinberg, could you chime in with what you are thinking?

(Added a WIP tag so this doesn't get merged until we sort this out. )

[PeterDSteinberg]

I was thinking a default econ_params.json file in the taxcalc repo and a corresponding EconParams class. Then the Calculator object initialization can take separate tax params and econ params dictionary arguments. Internal to Calculator objects, the tax params will remain accessible in .params and the economic parameters will be accessible at .econ_params, keeping the namespaces separate.

[martinholmer]

@PeterDSteinberg said:

I was thinking a default econ_params.json file in the taxcalc repo and a corresponding EconParams class. Then the Calculator object initialization can take separate tax params and econ params dictionary arguments. Internal to Calculator objects, the tax params will remain accessible in .params and the economic parameters will be accessible at .econ_params, keeping the namespaces separate.

I like the idea of a separate class, but I'm not crazy about your proposed names. The name econ_params.json makes everybody wonder what's in params.json. (Honestly, "parameters" is over-used in this project: everything can be viewed as a collection of parameters. If I had my way, the params.json file would be called current_law_policy.json and the Parameters class would be called Policy. But I wasn't active in the early days when this decision was made, and there's no going back now. However, that does not mean we have to make the same mistake again.) The contents of your proposed JSON file and class are BEHAVIORAL ASSUMPTIONS. Why not just build a Behavior class in the existing behavior.py file and call the default assumptions behavior.json? Much less confusion if you following this naming strategy.

[MattHJensen]

@martinholmer, @talumbau, @PeterDSteinberg

I plan to clean up pep8 and then merge this into master later today. Then Peter plans to refactor Parameters and Behavior to share some core functionality. In the meantime @Amy-Xu could start following this template for #368.

[MattHJensen]

@PeterDSteinberg I just renamed the Behavior method implement_reform() to update_behavior().

I also think that we should have one dictionary of reforms to pass to params.implement_reform() and a separate dictionary of new_behavior to pass to behavior.update_behavior(), and I went ahead and made this change throughout tests_behavior.py.

[MattHJensen]

@martinholmer, just want to make an off-topic comment before this PR gets closed and buried:

If I had my way, the params.json file would be called current_law_policy.json and the Parameters class would be called Policy

I don't think anyone would be averse to revisiting the naming scheme if you'd like to open an issue on this at some point.

[PeterDSteinberg]

Looks good to me!

[MattHJensen]

Merging. Thanks everyone.