-
Couldn't load subscription status.
- Fork 201
Add SNAP minimum benefit #315
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 SNAP minimum benefit #315
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know just a draft but a couple comments after checking it out
…to snap_min_benefit
…into snap_min_benefit
…te snap_min and snap final amount
|
I'm not sure why https://github.com/PolicyEngine/openfisca-us/runs/4419215730?check_suite_focus=true#step:5:591 |
|
@nikhilwoodruff could you take a look at this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just took a look @nmrodelo - it's what I think is a bug in Core (I've seen it before in a few places). Essentially, when we index a parameter with an array of numbers (e.g. amount_by_household_size[[1, 1, 1, 3, 1, 2]]) we get the right values, but when we index by just a single number (amount_by_household_size[1]) it doesn't seem to work - except when we do amount_by_household_size["1"]. Here's how I fixed it locally below.
Co-authored-by: Nikhil Woodruff <35577657+nikhilwoodruff@users.noreply.github.com>
|
@nikhilwoodruff is there anything left on my end for this PR? |
|
Try renaming |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also rename the test file to snap_minimum_benefit.yaml to match the variable name.
Codecov Report
@@ Coverage Diff @@
## master #315 +/- ##
=======================================
Coverage 94.87% 94.88%
=======================================
Files 59 59
Lines 3652 3654 +2
Branches 480 480
=======================================
+ Hits 3465 3467 +2
Misses 162 162
Partials 25 25
Continue to review full report at Codecov.
|
|
Thanks @nmrodelo, could you just do two more things? We're trying to get back on the http://semver.org track. You'll have to pull to fetch my latest PR.
I'll merge from there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…into pr/nmrodelo/315
…into pr/nmrodelo/315
|
@nmrodelo thanks for this PR! Just added the changelog entry, merging. |
Draft/ work in progress pull request for snap minimum benefit parameters