add the possibility to load the parameter value of a previous year. #306
Conversation
Codecov Report
@@ Coverage Diff @@
## grundrente #306 +/- ##
==============================================
+ Coverage 92.74% 92.81% +0.07%
==============================================
Files 70 71 +1
Lines 2783 2811 +28
==============================================
+ Hits 2581 2609 +28
Misses 202 202
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks @ChristianZimpelmann . I think your approach works for the grundrente case. I wonder however, whether we'd rather want to have a generic solution for every parameter. Right now, access_different_date needs to be specified for the parameter. Why not rather create a different set of parameters by invoking set_up_policy_environment() with a different year?
gettsim/policy_environment.py
Outdated
| # Also load earlier parameter values if this is specified in yaml | ||
| if "access_different_date" in raw_group_data[param]: | ||
| if raw_group_data[param]["access_different_date"] == "vorjahr": | ||
| date_last_year = date - datetime.timedelta(days=365) |
There was a problem hiding this comment.
this is not robust to leap years, isn't it? I'd suggest something like
date_last_year = date(date.year - 1, date.month, date.day)There was a problem hiding this comment.
good point, will adjust. But I think your solution also needs to take care of leap years in case the current date is Februar 29th
gettsim/tests/test_synthetic.py
Outdated
| # from gettsim import compute_taxes_and_transfers | ||
| # from gettsim import set_up_policy_environment | ||
| # from gettsim.synthetic_data.synthetic import create_synthetic_data | ||
| # def test_synthetic(): |
There was a problem hiding this comment.
this test should be reactivated before merging
There was a problem hiding this comment.
This PR will be merged to the grundrente PR. I will definitelly reactivate it before merging to main branch.
There was a problem hiding this comment.
Test should run through now
This wholly depends on how often we are likely to need it. Invoking the whole beast is costly every time 'round. Do (m)any other instances come to mind where we'd need the parameters of the previous year? In most cases, it seems to me like a question of individual data... (e.g., ALG I benefits and the like). Thoughts, @Eric-Sommer @mjbloemer ? |
|
Thank you for your comments, Eric!
I think we should hide this functionality from the user. Therefore, we shouldn't require them to regularly call Conversely, calling The two alternatives I was thinking about were:
Since we will only need this for a small subset of parameters, I decided for the first solution. |
|
Thank you very much, @Eric-Sommer 😄 |
Closes #305
Add the possibility to load the value of a parameter of the previous year when calling
set_up_policy_environment