PSLmodels / Business-Taxation

USA Corporate and Pass-Through Business Tax Model
11 stars 10 forks source link

How to construct an Asset object with a response? #68

Closed martinholmer closed 5 years ago

martinholmer commented 5 years ago

Here is the Asset constructor:

    def __init__(self, btax_params, corp=True, data=None, response=None):
        # Create an associated Data object
        if isinstance(data, Data):
            self.data = data
        else:
            self.data = Data()
        # Check inputs
        if isinstance(corp, bool):
            self.corp = corp
        else:
            raise ValueError('corp must be True or False')
        self.response = response     <-----------------------------------------------------
        if corp:
            self.adjustments = {'bonus': 0.60290131, 'sec179': 0.016687178,
                                'overall': self.data.adjfactor_dep_corp,
                                'rescalar': self.data.rescale_corp}
        else:
            self.adjustments = {'bonus': 0.453683778, 'sec179': 0.17299506,
                                'overall': self.data.adjfactor_dep_noncorp,
                                'rescalar': self.data.rescale_noncorp}
        if isinstance(btax_params, pd.DataFrame):
            self.btax_params = btax_params
        else:
            raise ValueError('btax_params must be DataFrame')

The above code does not check the type of the response argument (which is a clear code improvement opportunity).

How can I generated an object to pass to the Asset constructor via the response argument?

codykallen commented 5 years ago

@martinholmer noted:

The above code does not check the type of the response argument (which is a clear code improvement opportunity).

That's a good point. We should replace the response line with something like:

if response is not None and isinstance(response, pd.DataFrame):
    raise ValueError('response' must be DataFrame or or None')
self.response = response

If you want to pass the appropriate object, you'll have to use a BusinessModel object to run the reforms first:

bm.update_mtrlists()
bm.response = Response(self.elast_dict, self.btax_params_base, self.btax_params_ref)
bm.response.calc_all()
asset1 = Asset(btax_params = bm.btax_params_ref, response=bm.response.investment_response)

Essentially, I built in 2 ways to implement the responses, one when the object is created and the other by updating the object. Business-Taxation uses both of these. For example, it updates the Asset objects with the investment responses but it creates entirely new debt objects because the debt forecast relies on the debt response and the asset forecast, which first requires applying the investment response.

martinholmer commented 5 years ago

@codykallen said in issue #68:

We should replace the response line with something like:

if response is not None and isinstance(response, pd.DataFrame):
   raise ValueError('response' must be DataFrame or or None')
self.response = response

OK, that makes sense. Do you want to make that change in a separate pull request?

Watch out for the "double or" typo in the ValueError message.