iza-institute-of-labor-economics / gettsim

The GErman Taxes and Transfers SIMulator
https://gettsim.readthedocs.io/
GNU Affero General Public License v3.0
56 stars 33 forks source link

refactor: rename `hh_id` to `vg_id` #654

Closed lars-reimann closed 1 year ago

lars-reimann commented 1 year ago

What problem do you want to solve?

As discussed in https://github.com/iza-institute-of-labor-economics/gettsim/pull/601#issuecomment-1677783059, this PR

Since this PR touches thousands of files, I've split this from #601.

Todo

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 97.70% and no project coverage change.

Comparison is base (affd33c) 90.61% compared to head (0303e2f) 90.61%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #654 +/- ## ======================================= Coverage 90.61% 90.61% ======================================= Files 48 48 Lines 3100 3100 ======================================= Hits 2809 2809 Misses 291 291 ``` | [Files Changed](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics) | Coverage Δ | | |---|---|---| | [src/\_gettsim/config.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL2NvbmZpZy5weQ==) | `33.33% <ø> (ø)` | | | [src/\_gettsim/taxes/lohn\_st.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL3RheGVzL2xvaG5fc3QucHk=) | `86.07% <ø> (ø)` | | | [src/\_gettsim/time\_conversion.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL3RpbWVfY29udmVyc2lvbi5weQ==) | `64.17% <ø> (ø)` | | | [...ettsim/transfers/kinderzuschl/kinderzuschl\_eink.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL3RyYW5zZmVycy9raW5kZXJ6dXNjaGwva2luZGVyenVzY2hsX2VpbmsucHk=) | `100.00% <ø> (ø)` | | | [src/\_gettsim/transfers/kinderzuschl/kost\_unterk.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL3RyYW5zZmVycy9raW5kZXJ6dXNjaGwva29zdF91bnRlcmsucHk=) | `88.23% <60.00%> (ø)` | | | [.../\_gettsim/transfers/arbeitsl\_geld\_2/kost\_unterk.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL3RyYW5zZmVycy9hcmJlaXRzbF9nZWxkXzIva29zdF91bnRlcmsucHk=) | `95.45% <92.30%> (ø)` | | | [src/\_gettsim/aggregation\_numpy.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL2FnZ3JlZ2F0aW9uX251bXB5LnB5) | `76.82% <100.00%> (ø)` | | | [src/\_gettsim/demographic\_vars.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL2RlbW9ncmFwaGljX3ZhcnMucHk=) | `100.00% <100.00%> (ø)` | | | [src/\_gettsim/interface.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL2ludGVyZmFjZS5weQ==) | `83.33% <100.00%> (ø)` | | | [src/\_gettsim/synthetic.py](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics#diff-c3JjL19nZXR0c2ltL3N5bnRoZXRpYy5weQ==) | `98.48% <100.00%> (ø)` | | | ... and [8 more](https://app.codecov.io/gh/iza-institute-of-labor-economics/gettsim/pull/654?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=iza-institute-of-labor-economics) | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hmgaudecker commented 1 year ago

Thanks! Will need to alert some people before merging, but looks great at a glance!

Small things that I saw right away:

lars-reimann commented 1 year ago

We should document somewhere the origin of vg. Probably GEP-01 ?

What should be written there?

hmgaudecker commented 1 year ago

We should document somewhere the origin of vg. Probably GEP-01 ?

What should be written there?

Now that you are saying it, it is actually written already :see_no_evil:, right here... Apologies.

Reading through that makes me wonder whether this PR is actually helpful or not in its current form. That is, current hh will become either vg, or fg, or bg.

It might be more useful to use the correct naming already, leave all three of {vg, fg, bg} the same (=same behaviour as current hh) and then introduce the distinctions made in #601 ?

If that makes sense, it would be

Does that make sense, @mjbloemer @ChristianZimpelmann @JuergenWiemers ?

ChristianZimpelmann commented 1 year ago

Does that make sense,

I would say so!

Not really sure about grundrente. Probably fg? It could be the case even that we need another grouping there as I am not sure whether there is ever more than one generation considered.

A tricky part will be the Günstigerprüfung between Wohngeld and ALG II if those are referring to different groupings. But this isn't relevant in this PR.

lars-reimann commented 1 year ago

@hmgaudecker How should the following functions be renamed?

hmgaudecker commented 1 year ago

The last might actually pose a problem because wohngeld operates on different grouping than arbeitsl_geld_2, kinderzuschl, which we won't be able to solve before #601 and maybe a follow-up.

Just so you are not surprised.

lars-reimann commented 1 year ago

Closing since main has already diverged too far from this.