Skip to content

Conversation

@nmrodelo
Copy link
Contributor

Add variable and parameters for USDA disabled status

Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a test with the formula

Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also rename usda_disabled.yaml to disabled.yaml

nmrodelo and others added 3 commits December 1, 2021 11:34
Co-authored-by: Nikhil Woodruff <35577657+nikhilwoodruff@users.noreply.github.com>
@nmrodelo nmrodelo marked this pull request as ready for review December 1, 2021 19:36
Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikhilwoodruff note that we moved some fields around and included a float, ssdi alongside the bools. The tests indicate that the previous code still works, i.e. any is capturing ssdi>0.

@nikhilwoodruff
Copy link
Collaborator

@nikhilwoodruff note that we moved some fields around and included a float, ssdi alongside the bools. The tests indicate that the previous code still works, i.e. any is capturing ssdi>0.

It'd work here because OpenFisca's test runner automatically parses single-person arrays into bools. But since any isn't vectorised, I think it'd fail with multiple outputs, as any would return True or False, rather than an array.

Copy link
Contributor

@MaxGhenis MaxGhenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple final suggestions then we're good to merge

@MaxGhenis MaxGhenis merged commit 8ccf80b into PolicyEngine:master Dec 4, 2021
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