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.

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

Add iPython notebooks on Better Way reform (Ryan-Brady plan) #824

Closed
wants to merge 6 commits into from

Conversation

codykallen
Copy link
Contributor

If agi - ided > 0, delta_income = delta_wage + delta_oinc - delta_ided. I corrected the assert statement to reflect that.
If agi > 0 but agi_m_ided < 0, then delta_wage=delta_oinc=delta_ided=0 but delta_income != 0. I changed the order of commands and set delta_income=np.where(agi_m_ided>0, taxinc_change, 0.)

These changes eliminate the assertion errors in this function.

@Amy-Xu
Copy link
Member

Amy-Xu commented Jul 14, 2016

@martinholmer @MattHJensen Cody found this assertion error when he used the substitution and cap gain behavior functions. Could you review?

@martinholmer Just in case you haven't met Cody yet, he's a new RA at AEI, mainly focusing on tax policy.

@martinholmer
Copy link
Collaborator

@codykallen, Thanks for your proposed fix of logic in the behavior.py _update_ordinary_income function. Also, welcome to the OSPC team!

What we need is a new test in taxcalc/tests/test_behavior.py that fails on the master branch and passes with your fix of the behavior.py code. Your notebook will never be run by the Travis-CI system that runs our unit tests with ever GitHub commit. So, drop the notebook from this pull request and add the simplest code that triggers the assertion error you came across as a new test in test_behavior.py.

Does that make sense? If not, feel free to ask questions here.

@MattHJensen @Amy-Xu

@martinholmer martinholmer mentioned this pull request Jul 25, 2016
@martinholmer
Copy link
Collaborator

@codykallen, I have fixed in pull request #846 the two bugs you reported in pull request #824.

Thanks again for reporting these two bugs and suggesting coding changes that would fix them.

What you need to do with your intro1 branch is to (1) revert the changes in behavior.py and (2) merge the newest master branch (after syncing with GitHub) into your intro1 branch. Doing those two things will give you #846 and many other recent improvements on the master branch.

Do step (1) with the following command:

git checkout -- taxcalc/behavior.py

Do step (2) with the following command:

git merge master

@MattHJensen @Amy-Xu @GoFroggyRun

@martinholmer
Copy link
Collaborator

@codykallen, Now that pull request #846 has been merged into the master branch, you should change the name of pull request #824. Perhaps something like: "Add iPython notebook on Better Way reform".

@MattHJensen

@codykallen codykallen changed the title Correction to behavior.py function _update_ordinary_income Add iPython notebooks on Better Way reform (Ryan-Brady plan) Aug 1, 2016
@codykallen codykallen closed this Aug 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants