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

Convert default values for boolean parameters from 1/0 to "True"/"False" #788

Merged
merged 4 commits into from Dec 22, 2017

Conversation

hdoupe
Copy link
Collaborator

@hdoupe hdoupe commented Dec 21, 2017

This PR converts 1/0 values back to boolean variables and serializes them. PolicyBrain reads the default GUI parameter values from Tax-Calculator's default_data function. This line in parameters.py converts boolean parameters from true/false to 1/0. This treatment is currently only applied to DependentCredit_before_CTC but will be applied to the other comma separated boolean variables as they are added.

Before this PR, the default parameter value for DependentCredit_before_CTC was shown as:
screen shot 2017-12-21 at 5 52 17 pm

Behavior introduced by this PR shows:
screen shot 2017-12-21 at 5 53 14 pm

@hdoupe
Copy link
Collaborator Author

hdoupe commented Dec 21, 2017

PR #788 is on the test-app. I hope to merge this and push to production tomorrow if there are no objections.

@MattHJensen
Copy link
Contributor

MattHJensen commented Dec 22, 2017

@hdoupe, I just tried entering False, False, False and got an error. Same with True, True, True

Expected case insensitive 'true' or 'false' but got True

Looks like the problem is the spaces after the commas, which are fine with other parameters. False,False,False is just fine.

@hdoupe hdoupe mentioned this pull request Dec 22, 2017
@hdoupe
Copy link
Collaborator Author

hdoupe commented Dec 22, 2017

Thanks for the bug report @MattHJensen. I just updated the test app with the bug fix.

@MattHJensen
Copy link
Contributor

@hdoupe, everything is looking good to me!

@hdoupe hdoupe merged commit f83af8e into ospc-org:master Dec 22, 2017
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.

None yet

2 participants