[1] - added tests for winners_and_losers #32
Conversation
masterismail
changed the title
There was a problem hiding this comment.
Thanks for this, @masterismail. Most of this looks good, but I want to confirm one thing. For the winners and losers and deciles tests, it looks like you code a particular tax change, then check the decile output against a hard-coded series of values.
If this is correct, this would make the tests extremely fragile. Because of the complex interaction effects tax policy creates, it wouldn't be surprising if these tests failed, say, a few months down the road because the UK changed how it calculated some particular benefit program. If that's the case, I'd ask that you rewrite these, but I'm not quite sure how yet - I'll leave that in a separate comment.
|
What I would instead do, if possible, is inject particular values that you expect to return a particular result. For example, for |
|
Changing the status of this to |
|
We also don't want the test suite to require restricted data. Would this run over a synthetic file? Or how about creating a simple fake country system for these tests? @nikhilwoodruff |
|
Absolutely, @anth-volk. You’re right—I had a similar conversation with Nikhil about this. We also recognized the fragility of these tests and mentioned it during our meeting. The current tests were really just a temporary solution to ensure that the overall package workflow was functioning correctly for the metrics we defined. given that we also need to consider that the test suite don't have restricted data, creating a specific country system for testing is a good idea. It could also benefit other packages. Thanks so much for bringing this up! |
...gine/tests/economic_impact/winners_and_losers/test_by_wealth_decile/test_by_wealth_decile.py
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,19 @@ | |||
| # Winners and Losers by income | |||
There was a problem hiding this comment.
Can we change this to a US test?
There was a problem hiding this comment.
Is there any specific parameter you would recommend that could impact our target metrics ?
There was a problem hiding this comment.
This one- see reproduce in python for the parameter name. https://policyengine.org/us/policy?reform=51488&focus=gov.irs.credits.ctc.refundable.fully_refundable®ion=us&timePeriod=2024&baseline=2
There was a problem hiding this comment.
updated with the requested parameter, although we still have zeroes at some places.
No description provided.