[WIP] Add organ donor compensation by MattiSG · Pull Request #16 · BetterRules/openfisca-aotearoa

MattiSG · 2018-05-20T04:26:25Z

Tax and benefit system evolution.
Impacted periods: all.
Impacted areas: live_organ_donor_compensation
Details:
- Add computation for live organ donor compensation.

These changes:

Add a new feature (for instance adding a variable)

These changes were made during the Better Rules Tech Week hackathon.

Basic scenario working

* tests: Changed to 52 week calculation

Make reference actually make sense

Update live_organ_donor_compensation.py

MattiSG

Excellent! That seems almost mergeable as such, congratulations for such a clean and tested implementation! I was really impressed with the organiccoders.allengeer.com URIs as references, with proper semantics 👏 Do you expect these URLs to be stable over time?

Since I opened this PR I cannot approve it. There are a couple comments I made you might want to address, and we'll need to update the changelog, but other than that this seems good to go!

Are you expecting to add some major functionality in there or are you be happy with the current state of things? 😃

MattiSG · 2018-05-21T04:53:39Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+    value_type = float
+    entity = Person
+    definition_period = YEAR # TODO - determine whether we need to get WEEK to work
+    label = u"The amount deducted as a kiwisaver contribution"


Adding a legislative link as a reference property to these variables would help a lot in double-checking the implementation 🙂

MattiSG · 2018-05-21T04:54:52Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+#     value_type = float
+#     entity = Person
+#     definition_period = YEAR # TODO - determine whether we need to get WEEK to work
+#     label = u"The salary earned by a person before tax"


Version control (Git) can help us recover code if we need it, but commented-out code usually becomes a problem over time as we cannot know if it is still up-to-date or not… Could we maybe remove these parts from the model? 🤔

MattiSG · 2018-05-21T04:56:25Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+class weekly_compensation_before_tax(Variable):
+    value_type = float
+    entity = Person
+    definition_period = YEAR # TODO - determine whether we need to get WEEK to work


Is this comment still relevant? It seems this code works well with YEAR, even though it would be more proper with WEEK.

In the current scheme, ETERNITY may be better since the value doesn't correspond to a specific MONTH or YEAR.

I think the value actually corresponds to a particular point in time corresponding to the date of the latest pay period that has been used in the calculation.

MattiSG · 2018-05-21T05:03:04Z

CircleCI is still waiting because building forks was not activated yet. I just switched it on, any commit on this branch will trigger a build 🙂

* 'master' of github.com:OrganicCoders/openfisca-aotearoa: Update live_organ_donor_compensation.py

nigelcharman

Please do not merge yet. I think there's more discussion to be had about the implementation.

The organiccoders.allengeer.com URLs are impermanent, with the server likely to be shut down soon. We need to consider where these might transition to?

Also it is far from complete in terms of the legislation. It is currently missing shareholder employee and overseas earners, and could do with a review by someone more familiar with the legislation if it is to be used in earnest.

Ideally we would have modules such as Income Tax, Student Loans, KiwiSaver etc that we could call to make this more complete.

We test drove the implementation so it was automatically well tested :)

nigelcharman · 2018-05-21T10:34:31Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+class weekly_compensation_before_tax(Variable):
+    value_type = float
+    entity = Person
+    definition_period = YEAR # TODO - determine whether we need to get WEEK to work


In the current scheme, ETERNITY may be better since the value doesn't correspond to a specific MONTH or YEAR.

I think the value actually corresponds to a particular point in time corresponding to the date of the latest pay period that has been used in the calculation.

nigelcharman · 2018-05-21T10:35:36Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+    value_type = float
+    entity = Person
+    definition_period = YEAR # TODO - determine whether we need to get WEEK to work
+    label = u"The amount deducted as a kiwisaver contribution"


nigelcharman · 2018-05-21T10:37:55Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+        return round(employee_weekly_earnings + self_employed_weekly_earnings, 2)
+
+class kiwisaver_employee_deduction(Variable):
+    value_type = float


This method might be better in a KiwiSaver specific module, so that it picks up any changes to KiwiSaver.

It's also largely copy and paste from above. It should utilise the method from above to calculate the basic compensation and then calculate KiwiSaver on that amount.

It is also missing concepts such as KiwiSaver holiday period.

Br3nda · 2018-05-21T10:52:05Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

+    value_type = int
+    default_value = 0
+    entity = Person
+    label = u"Kiwisaver employee deduction percentage "


Extra space in end of string here

Fixed, thanks

MattiSG · 2018-05-22T02:41:48Z

Thanks a lot for all these changes already! 👏

where these might transition to?

The best would probably be legislation.govt.nz 🙂 If there is no better reference, this is still much better than nothing!

it is far from complete in terms of the legislation

This will always be the case. We never know what we don't know from the legislation. This is already the most accurate model available 🙂 It is fine to proceed with this PR and add more details in further ones.

BrigetteM · 2018-05-23T02:23:25Z

re the URLs as persistent ids, I think that given we couldn't execute them as proper URIs and have no one to manage these persistent ids, that we don't have much choice than to point to the legislation. The problem with this however, is that this URL will change over time, and so it is not a good long term solution. It points to a piece of work that in Aus our working group does for free, which perhaps doesn't exist here- that is, management of persistent ids. in Aus, the Linked Data working group have linked.data.gov.au and that runs alongside data.gov.au, giving us a home from which to manage this stuff. The info and link to the Persistent Id service is listed on the showcase page here. Something for the team to work through perhaps.

nigelcharman · 2018-05-23T06:01:40Z

Thanks @BrigetteM. I can see how linked.data.gov.au would help with this. Hope you've shown that to Pia and co.
In the meantime, to preserve the work you did on the linked data definitions for this project, I've added to OrganicCoders#4. As noted on the PR, it's not optimal since the URL contains the repository and branch name, so I see this as a step towards a more permanent home.

Added definitions of 'number of weeks' and 'earnings as an employee'

nigelcharman · 2018-05-23T08:08:22Z

I've updated this PR with:

New definitions folder containing definitions for 'number of weeks' and 'earnings as an employee'
Updated the references in variables/live_organ_donor_compensation.py to reference these definitions

Note that if we go with this and this pull request is actioned, we will need to update the repository name in the reference URLs. There could be a way to resolve this in future using something like python-feedparser in the pipeline to resolve relative links into actual links, but it would be better to move to a permanent site such as linked.data.gov.au (see above).

MattiSG

The idea of adding “definitions” straight in the OpenFisca is very thought-provoking. I find it very interesting, but I feel that it also bring quite a commitment to maintain them.
@verbman @Br3nda what do you think of that? Should we bring an ontology into OpenFisca Aotearoa?

MattiSG · 2018-05-28T04:03:44Z

openfisca_aotearoa/variables/live_organ_donor_compensation.py

    entity = Person
    label = u"Total earnings over last 52 weeks"
-    reference = "http://organiccoders.allengeer.com/brk-1234-abcdfcrg234356/"
+    reference = "https://github.com/OrganicCoders/openfisca-aotearoa/blob/master/openfisca_aotearoa/definitions/earnings_as_an_employee.md"


This is not a stable URL: if the file gets renamed it would yield a 404. You can obtain a stable URL by navigating to this URL and pressing y on your keyboard 🙂

OpenFisca Aotearoa (poly?) to ensure tests pass after OpenFisca Core upgrade

nigelcharman and others added 16 commits May 19, 2018 12:42

First basic test

7818fa5

Added tax code and after tax amount

0d6a6f7

Commented out tax implications

fbe6036

Initial test passing

cc92a5c

Tidy up

090f12d

Merge pull request #2 from OrganicCoders/tests

2cd9f23

Basic scenario working

Changed to 52 week calculation

6800008

Merge branch 'tests'

2995ce2

* tests: Changed to 52 week calculation

Applied rounding, commented out unneeded code

0f810b5

Added subpart 1 5(3) compensation earnings disregarded

b472181

Added self-employed earnings

b2d7db2

Added kiwisaver

dbbfcf6

Added reference to definition

bc00352

Added reference to definition

36822d7

Update live_organ_donor_compensation.py

b958b43

Make reference actually make sense

Merge pull request #3 from allengeer/patch-1

e68ff34

Update live_organ_donor_compensation.py

MattiSG mentioned this pull request May 21, 2018

OpenFisca support for weekly periods OrganicCoders/openfisca-aotearoa#1

Closed

MattiSG commented May 21, 2018

View reviewed changes

MattiSG requested a review from verbman May 21, 2018 05:03

nigelcharman added 6 commits May 21, 2018 20:37

Removed commented out code

0c7ee37

Added references to legislation

067d5cb

Fixed comments and removed kiwisaver_member variable

f138396

Merge branch 'master' of github.com:OrganicCoders/openfisca-aotearoa

429becd

* 'master' of github.com:OrganicCoders/openfisca-aotearoa: Update live_organ_donor_compensation.py

Added docstring comment

2e507ed

Fixed formatting so tests pass when using 'make test'

82a8810

nigelcharman suggested changes May 21, 2018

View reviewed changes

Added reference to KiwiSaver rate

d78b98f

Br3nda reviewed May 21, 2018

View reviewed changes

Removed space at end of string

2aaaf2b

nigelcharman mentioned this pull request May 22, 2018

Add support for other time periods openfisca/openfisca-core#670

Closed

nigelcharman added 2 commits May 23, 2018 17:33

Added definitions of 'number of weeks' and 'earnings as an employee'

5fbe05f

Updated references to definitions

64c5842

nigelcharman added 2 commits May 23, 2018 19:59

Merge pull request #4 from OrganicCoders/data-definitions

74b15f3

Added definitions of 'number of weeks' and 'earnings as an employee'

Updated references to definitions on master branch

21e69e5

MattiSG commented May 28, 2018

View reviewed changes

Br3nda changed the title ~~Add organ donor compensation~~ [WIP] Add organ donor compensation Aug 12, 2018

verbman removed their request for review October 8, 2018 03:32

verbman added a commit that referenced this pull request May 7, 2024

Merge pull request #16 from govzeroaotearoa/testmask

647b31f

OpenFisca Aotearoa (poly?) to ensure tests pass after OpenFisca Core upgrade

Conversation

MattiSG commented May 20, 2018

Uh oh!

MattiSG left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MattiSG commented May 21, 2018

Uh oh!

nigelcharman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MattiSG commented May 22, 2018

Uh oh!

BrigetteM commented May 23, 2018

Uh oh!

nigelcharman commented May 23, 2018

Uh oh!

nigelcharman commented May 23, 2018

Uh oh!

MattiSG left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants