GUI input types

[hdoupe]

In issue #782, @MattHJensen commented:

Just my 2 cents, I am more eager for TaxBrain to be able to accept True/False than for either TaxBrain or Tax-Calculator to accept 1.0, 1, 0, 0.0. We just have so many numeric inputs, including switches, it refreshing to differentiate switches.

The current behavior for the new comma separated boolean value allows for case-insensitive "true" and "false" as well as 1.0, 1, 0.0, and 0. These values are then converted into Python boolean type variables (True or False).

It seems like there are a couple different options for the new comma separated boolean parameters:

1. (current behavior) allow any input close to boolean (true/1/1.0 or false/0/0.0), convert that to boolean type True or False variable, and send the python boolean type variable to Tax-Calculator
2. allow any input close to boolean (true/1/1.0 or false/0/0.0), convert strings to python type ("false" --> False or "1" --> 1), and send the converted values to Tax-Calculator
3. only allow case insensitive true/false (includes True, TRUE, etc.) and 1, 1.0, 0, 0.0 are not valid GUI inputs, convert the input string to python boolean type and send to Tax-Calculator

I prefer (2). I presented my reasoning in PSLmodels/Tax-Calculator#1782:

I implemented a temporary fix [Note: this "temporary fix" is current behavior] in this commit. This fix will be removed once the next version of Tax-Calculator is released.

The fix isn't necessarily bad, but it does introduce unneeded complexity. Further, I think it makes PolicyBrain a little too opinionated. Past experience suggests that parameter checking and conversion at this level should be left to taxcalc.

However, I would like to hear the perspectives of others on this issue.

[martinholmer]

@hdoupe, Tax-Calculator pull request 1782 has been merged. If it would make you work on PolicyBrain any easier, we could make a new 0.14.3 bug-fix release right away. Would that be helpful or not?

[hdoupe]

@martinholmer I think you can wait and wrap 1782 into the next regularly scheduled Tax-Calculator release. I already implemented and merged a solution to this issue. This solution is either a long-term fix or a temporary work around depending on whether we decide to go with 1, 2, or 3 above.

@martinholmer I'd be interested in your thoughts on approaches 1, 2, or 3 as a I listed above. Essentially, the question is how much responsibility do we want PolicyBrain to have for input validation versus how much responsibility to we want Tax-Calculator (or some other model) to have for input validation.

[MattHJensen]

@hdoupe, I might be missing something, but isn't a fourth option to only allow inputs that are valid in Tax-Calculator? Based on my current understanding, your preferred approach of (2) still means having extra code in PolicyBrain for validation/conversion. Could we just require the user give an input that is valid for Tax-Calculator and return them a Tax-Calculator error if they don't?

[martinholmer]

@MattHJensen said:

I might be missing something, but isn't a fourth option to only allow inputs that are valid in Tax-Calculator? Based on my current understanding, your preferred approach of (2) still means having extra code in PolicyBrain for validation/conversion. Could we just require the user give an input that is valid for Tax-Calculator and return them a Tax-Calculator error if they don't?

The HEAD of the Tax-Calculator master branch considers True,1,1.0 or False,0,0.0 as valid values for boolean policy parameters, which is the way Python handles things (as pointed out by @hdoupe). This has been a useful change because it allows more flexibility in processing JSON reform files that are hand-written by experienced Python programmers working on their local computer, who then want to upload the reform file to TaxBrain.

It is a logically separate question about what the TaxBrain GUI should do. Requiring True or False in the TaxBrain GUI might be an advantage in that it emphasizes the type of the policy parameter. Allowing 1,1.0 or 0,0.0 in the GUI seems to assume more computer programming knowledge than most TaxBrain GUI users are likely to have. But no matter what is decided about the TaxBrain GUI, Tax-Calculator will be able to support the decision.

[MattHJensen]

@martinholmer, thanks for that explanation. That clears up my misunderstanding.

[hdoupe]

@martinholmer said

It is a logically separate question about what the TaxBrain GUI should do. Requiring True or False in the TaxBrain GUI might be an advantage in that it emphasizes the type of the policy parameter. Allowing 1,1.0 or 0,0.0 in the GUI seems to assume more computer programming knowledge than most TaxBrain GUI users are likely to have. But no matter what is decided about the TaxBrain GUI, Tax-Calculator will be able to support the decision.

I think TaxBrain should default to True or False but accept 1/0/1.0/0.0 as valid inputs. As @martinholmer pointed out, it is probably more intuitive to TaxBrain users to think about this type of parameter as True/False instead of 1/0. I think defaulting to True/False but accepting 1/0/1.0/0.0 along with the behavior in step 2 accomplishes a few things:

1. Defaulting to "true" or "false" strongly indicates that this parameter is "on" or "off."
2. Type checking beyond the bare minimum that is done by PolicyBrain is left completely to Tax-Calculator. If we limit the input to only True/False, then this something extra that PolicyBrain has to check.
   Note: PolicyBrain only checks to make sure text inputs are numbers or some variant of "true" or "false."

@MattHJensen So we would only allow inputs that are valid taxcalc inputs, but we would also provide a strong hint to indicate the type for the parameter. This does require a little extra code to parse the comma separated boolean strings. However, this code really just deserializes the strings. For example, it converts "1" to 1 or "false" to False.

[MattHJensen]

That makes sense. Thanks @hdoupe!

[martinholmer]

@hdoupe said:

I think TaxBrain should default to True or False but accept 1/0/1.0/0.0 as valid inputs. As @martinholmer pointed out, it is probably more intuitive to TaxBrain users to think about this type of parameter as True/False instead of 1/0. I think defaulting to True/False but accepting 1/0/1.0/0.0 along with the behavior in step 2 accomplishes a few things:

Defaulting to "true" or "false" strongly indicates that this parameter is "on" or "off."
Type checking beyond the bare minimum that is done by PolicyBrain is left completely to Tax-Calculator. If we limit the input to only True/False, then this something extra that PolicyBrain has to check.
Note: PolicyBrain only checks to make sure text inputs are numbers or some variant of "true" or "false."
@MattHJensen So we would only allow inputs that are valid taxcalc inputs, but we would also provide a strong hint to indicate the type for the parameter. This does require a little extra code to parse the comma separated boolean strings. However, this code really just deserializes the strings. For example, it converts "1" to 1 or "false" to False.

Fine, I don't have any strong feelings about how TaxBrain should work.

[hdoupe]

@martinholmer @MattHJensen Great, thanks for your feedback.

[hdoupe]

Closed via #822