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

Use Paramtools 'when' validator for spouse wages and age #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Peter-Metz
Copy link
Collaborator

The goal is to alert CS users that they must enter "Married" for filing status before entering spouse wages or age.

@hdoupe this is a great feature! Is there a way add a custom error message? Or will the CS user be alerted in real time that their input isn't valid?

@hdoupe
Copy link
Contributor

hdoupe commented Feb 1, 2021

@Peter-Metz this validation isn't done in real-time on the CS frontend. You can use the default ParamTools error message, or you can try to catch errors for the sage and swages parameters and add your own message.

@hdoupe
Copy link
Contributor

hdoupe commented Feb 1, 2021

this is a great feature!

Thanks dude! I'm glad to see it be used more!

@Peter-Metz
Copy link
Collaborator Author

Thanks Hank! What would the Paramtools default error message say in this case?

@hdoupe
Copy link
Contributor

hdoupe commented Feb 6, 2021

Hey @Peter-Metz , sorry for the delay on this. The error message would look like this:

image

@hdoupe
Copy link
Contributor

hdoupe commented Feb 6, 2021

It would be nice if ParamTools could use custom error message templates. For example, this is the template string for the when validator:

        error_then = (
            f"When {oth_param}{{when_labels}}{{ix}} is {{is_val}}, "
            f"{param_name}{{labels}}{{ix}} value is invalid: {{submsg}}"
        )
        error_otherwise = (
            f"When {oth_param}{{when_labels}}{{ix}} is not {{is_val}}, "
            f"{param_name}{{labels}}{{ix}} value is invalid: {{submsg}}"
        )

@jdebacker
Copy link
Member

@Peter-Metz and @hdoupe -- is this PR ready? If we are going to re-launch a Tax-Cruncher web application, how helpful would this be?

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.

3 participants