-
Notifications
You must be signed in to change notification settings - Fork 75
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
Align YAML tests and JSON API situation description #716
Comments
Solving this issue means defining a new format for YAML tests. Here are some thoughts:
Trying to balance these two principles, here is a proposition of specification for a new test format: |
RFC 1 - General structure of a YAML test- name: "Income tax - basic case"
description: ...
... # other test-level data, such as error margin, reforms to use, etc.
input:
... # Simulation input
output:
... # Expected output of the simulation The main change relatively to today's format is to:
|
RFC 2 - Entity-less modeThe following format is valid: - name: "Income tax - basic case"
input:
salary:
2018-10: 2000
output:
income_tax:
2018-10: 300 In this case, we make the assumption that:
|
RFC 3 - Simplified period modeThe following format is valid: - name: "Income tax - basic case"
period: 2018-10
input:
salary: 2000
output:
income_tax: 300 In this case, we make the assumption that:
|
RFC 3-alt - Simplified period mode (alternative)The following format is valid: - name: "Income tax - basic case"
input:
period: 2018-10
salary: 2000
output:
income_tax: 300 In this case, we make the same assumption than in 3, but the period is specified in a different place.
|
RFC 4 - Full entities specificationThe following format is valid: - name: "Income tax - couple"
period: 2017-01
input:
persons:
Alicia:
salary: 4000
Javier:
salary: 2500
households:
household_1:
parents: [Alicia, Javier]
output:
persons:
Alicia:
income_tax: 600
Javier:
income_tax: 375 This is almost a Web API-like JSON, except it's using RFC 3 to simplify periods. Of course, fully specifying periods for all input and output would work too. Note the repetition of |
RFC 5 - Full entities specification input, entity-less outputThe following format is valid: - name: "Total taxes - couple"
period: 2017-01
input:
persons:
Alicia:
salary: 4000
Javier:
salary: 2500
households:
household_1:
parents: [Alicia, Javier]
output:
total_taxes: 1050 When there is no ambiguity, the entity structure doesn't need to be repeated in the output.
|
RFC 6 - Single-entity simplified modeThe following format is valid: - name: "Total taxes - couple"
period: 2017-01
input:
persons:
Alicia:
salary: 4000
Javier:
salary: 2500
household:
parents: [Alicia, Javier]
output:
total_taxes: 1050 Using the singular name ( |
RFC 7 - Vectorial inputThe following format is valid: - name: "Income tax - different salaries"
period: 2018-10
input:
salary: [2000, 3000, 5000]
output:
income_tax: [300, 450, 750] In this case, if
|
RFC 8 - Partial entity specificationIn a tax and benefit system that includes two group entities, - name: "Housing tax - two owners"
period: 2017-01
input:
persons:
Alicia:
salary: 4000
Javier:
salary: 2500
titled_property:
owners: [Alicia, Javier]
accommodation_size: 100
output:
housing_tax: 1050 In this case, for consistency with RFC 7, and because we can't make general assumptions about relationships between entities, we make the assumption that:
|
@jvalduvieco, @dlouise64, @Br3nda, @verbman, @guillett, @Morendil, you are the one writing a lot of tests. Please let us know if:
We intend to move on relatively soon to start implementing, but if you think you have an opinion about this topic and need more time to think about it, let us know. Note that whatever choices we make here, migrations scripts will be provided to OpenFisca users. @jvalduvieco, you wanted to be able to use anchors and references in YAML tests, right? We could try to enable this in the PR to come. |
Thanks @fpagnoux for make easier the OpenFisca onboarding. A few comments:
|
Thanks @fpagnoux! For I'm only confused about |
@fpagnoux I owe you comments on this, but other things took priority, I'm not forgetting it. |
Thanks @fpagnoux !
I guess some of this pains might be avoided with a different design, opinions and guidance are welcome. |
It's not necessarily clear to everyone (it's not to me) what conclusions we've come to after this RFC, and from looking at WIP some of these options are being implemented which will make them de facto decisions, so I'd suggest a recap to make it clear where we are heading. Is there really a lot of value from changing What I'm seeing by looking at WIP is that this is, so far, the only change to some test files. But if many existing test files are already in conformity with the new format, and only some of the more complex ones need to be changed, I would strongly favor backward-compatible naming of the keys. This way large models like France are spared a large migration to the new format. |
I'm don't think we can spare the large migration to France, and this renaming is not the hard part 😓 .
Not a huge one, but it makes it really easy to check if a test has already been migrated, which has some advantages for the migration script.
|
Just fyi, we're hoping to do a visualisation of the tests, from the yaml, to help explain the concept to non coders. We'll delay until the new format is somewhat stabilised into core. |
Catching up on this thread, we use entities in many tests in Aotearoa (New Zealand's) openfisca rules. We have entitlements that an adult gets if they have, for example, a disabled child in their care. We also have some entitlements to a rebate for a property (a home) based on the occupants. Many other examples. Much of the law we've encoded so far is all about the relationships between entities. |
Gah hit close button by mistake |
Done 🎉 |
Right now, the description of a situation in a YAML test and in the Web API have a slightly different structure:
This is often confusing for users
The test runner also currently depends on Scenarios to parse these YAML files, while we've been trying to uncouple scenarios from the core of OpenFisca.
We should align the test structure to the API one, and process them the same way.
The text was updated successfully, but these errors were encountered: