Skip to content
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

Update openfisca-uk usage #11

Merged
merged 3 commits into from
Apr 26, 2021
Merged

Update openfisca-uk usage #11

merged 3 commits into from
Apr 26, 2021

Conversation

nikhilwoodruff
Copy link
Collaborator

OpenFisca-UK just had a considerable update merged, which uses a new interface (to make loading data from different surveys and years much easier, as well as performance improvements). I've adjusted the code here that accesses openfisca-uk to use the new API (largely just syntax changes). A few comments/questions on the pre-existing code, the semantics of which I haven't changed:

  • sim.calc is equivalent to sim.df([one_variable]).values.squeeze(), but more concise
  • the UK has National Insurance - is this close enough to US payroll tax?
  • self-employment: we use trading profits to calculate tax, but income drawn from the business for gross income (to be consistent with official disposable income definitions). Not sure if this was already known, just thought I'd double check
# Compute marginal tax rates (can only do on earned income now)
mtr = sim.mtr().values
  • Happy to say this is no longer the case! You'll want to use sim.deriv(target, wrt=source, group_limit=lim), where target is e.g. household net income, source is e.g. dividend_income, and group_limit is e.g. 2 (we calculate derivatives for individual adults, but preserving independence within households), and just negate it to get the format needed.

@codecov-commenter
Copy link

codecov-commenter commented Apr 23, 2021

Codecov Report

Merging #11 (0b9d8b7) into main (55f9f95) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
- Coverage   51.76%   51.63%   -0.14%     
==========================================
  Files           5        5              
  Lines         736      736              
==========================================
- Hits          381      380       -1     
- Misses        355      356       +1     
Flag Coverage Δ
unittests 51.63% <100.00%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
og_uk_calibrate/get_micro_data.py 57.14% <100.00%> (ø)
og_uk_calibrate/tests/test_get_micro_data.py 85.71% <100.00%> (ø)
og_uk_calibrate/tests/test_txfunc.py 70.68% <0.00%> (-0.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55f9f95...0b9d8b7. Read the comment docs.

@nikhilwoodruff
Copy link
Collaborator Author

@jdebacker @rickecon not really sure why this is specifically failing on 3.9? I've only really changed the API specifics, the inputs and outputs to the openfisca code should be identical. Could be an openfisca-uk issue.

@jdebacker
Copy link
Member

@nikhilwoodruff it's nothing you did. The PR adding openfisca UK also failed 3.9 but was merged before tests completed.

I don't know why it fails but you can remove 3.9 from test matrix if you like. I think that's ok since openfisca only tested on 3.7.

@rickecon
Copy link
Member

@jdebacker's OpenFisca-UK Integration PR #2 to this repo was passing all tests when I merged it. And build_and_test.yml tested Python 3.7, 3.8, and 3.9. I think @nikhilwoodruff is right that this issue was introduced with the update to OpenFisca-UK. But I think it is fine for now to remove Python 3.9 from the build_and_test.yml.

@rickecon
Copy link
Member

@nikhilwoodruff @jdebacker. I have reviewed this PR, and it looks good to me. Any further changes from either of you two?

@nikhilwoodruff
Copy link
Collaborator Author

Thanks @rickecon - nothing further to add here from me.

@rickecon rickecon merged commit 5df4d9b into PSLmodels:main Apr 26, 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.

None yet

4 participants