[WIP] Add and test, but do not use, Policy class

[martinholmer]

WORK IN PROGRESS

This pull request adds and tests a new Policy class, but this pull request does not yet attempt to use the new Policy class in the Business-Taxation code. All the old tests continue to pass and the example.py script continues to generates the same results.

One of the new tests in test_policy.py shows that the default policy parameter values read by the Policy class from the new policy_current_law.json can be converted into an all-year parameters DataFrame (using the Policy class parameters_dataframe method) that is identical to the DataFrame read from the existing mini_params_btax.csv file.

In an unrelated development, the Data class has been refactored so that the read_csv function is now part of the Data class, which increases the portability and usefulness of the class.

[martinholmer]

@codykallen, I found the use of an invalid depreciation method, EconomicDS, in reform1, which is used in many of the tests. This problem dates back to, at least, the merge of pull request #50 on 05-Jan-2019 (see this line of code). This method looks to be invalid because there is nowhere in the code that handle this depreciation method. But the error checking logic in the current code didn't catch the use of an unknown depreciation method.

However, the use of EconomicDS would be caught by the new Policy class when it goes into use. So, we need to somehow resolve this issue before proceeding with the use of the new Policy class.

By doing some execution tracing, I found that in the misspecified reform1 the current code actually leaves the 25-year depreciation method value at GDS in 2017+. So, commit 4e45ad7 in this pull request fixes the specification of reform1 in a way that leaves all the test results unchanged.

[martinholmer]

@codykallen, I forgot to mention that the code in pull request #74 requires the new taxcalc 1.2.0 package, which will be available on late Saturday (or sometime Sunday 24-Mar-2019 at the latest).

[codykallen]

@martinholmer said:

I found the use of an invalid depreciation method, EconomicDS, in reform1, which is used in many of the tests.

That's a good catch. It should say Economic. The reason it wasn't caught is that the logic of the Asset class uses GDS as the default. See the code here. If the specified method is not 'ADS', 'Economic' or 'None', then it uses GDS as the default.

[martinholmer]

@codykallen said in PR #74:

That's a good catch. It should say Economic. The reason it wasn't caught is that the logic of the Asset class uses GDS as the default. See the code here. If the specified method is not 'ADS', 'Economic' or 'None', then it uses GDS as the default.

But that doesn't agree with this code in the brcparams_preTCJA.json file.

Which file is correct?

If the code is correct, why is there a brcparams_preTCJA.json file? Should it be deleted? And if the code is correct, what does None mean? No depreciation? I don't understand.

I can't continue without some clarification on this.

[codykallen]

@martinholmer asked:

If the code is correct, why is there a brcparams_preTCJA.json file?

I had previously intended to switch from using a CSV of parameters to a JSON, but had only partially filled out the JSON for it. Although brcparams_preTCJA.json is in Business-Taxation, it isn't in use.

@martinholmer also asked:

And if the code is correct, what does None mean? No depreciation?

That's correct. There are 2 asset types that do not receive depreciation deductions or any capital cost recovery methods. These are Land and Inventories.

[martinholmer]

@codykallen said in PR #74:

There are 2 asset types that do not receive depreciation deductions or any capital cost recovery methods. These are Land and Inventories.

Now, I'm even more confused. The depreciation method string parameter refers to asset classes by year as in this code. Land and inventories are not asset year classes are they? If so, which year are they? Is land in the 39-year class? If so, it has under current-law a GDS method of depreciation.

When you drafted the brcparams_preTCJA.json file, you seemed to allow for an Expensing method of depreciation. And that method is used in test reform1 as you can see in this code.

It looks to me as if the code in the Asset class has an error because Expensing is not handled. Is that what the Asset code means by None? No depreciation because the asset is completely depreciated in the year it was purchased.

[martinholmer]

Closing PR #74 in favor of PR #81.