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

Add editorial content on parameter and variable #53

Merged
merged 7 commits into from
Oct 2, 2018

Conversation

sandcha
Copy link
Contributor

@sandcha sandcha commented Sep 13, 2018

Connected to openfisca/legislation-explorer#124

Note :
This branch was necessary to test openfisca/openfisca-core#717
Nevertheless, the question is: what do we want to keep as documentation example for new countries? 🙂


  • Minor change
  • Impacted areas: parameters/benefits, parameters/benefits/housing_allowance and variables/benefits
  • Details:
    • Add documentation to parameters: benefits node and benefits/housing_allowance
    • Add documentation to housing_allowance variable and docstring to its formula

These changes:

  • Improve already existing parameter and variable metadata.

@sandcha sandcha changed the title [WIP] Add editorial content on parameter and variable Add editorial content on parameter and variable Sep 14, 2018

# This allowance was introduced on the 1st of Jan 1980. Calculating it before this date will always return the variable default value, 0.
def formula_1980(household, period, parameters):
'''This is my specific life statement.'''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this sentence.
Realistic examples (instead of "my variable"-like ones) are easier to understand :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

  • In the variable documentation, mention that this benefit appeared in 1980 and disappeared in Dec 2016.
  • In the formula doc, mention the dependency on 'rent'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I agree on realistic examples. I simply lacked inspiration. 🙂

unit = 'currency-EUR'
documentation = '''
This allowance was introduced on the 1st of Jan 1980.
It needs the 'rent' value (same month) but doesn't care about the 'housing_occupancy_status'.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍, this kind of information adds a lot of value I think.
I'd maybe explicit a little more what we mean by "It needs":

To compute this allowance, the 'rent' value must be provided for the same month, but ''housing_occupancy_status'' is not necessary

@@ -1,3 +1,4 @@
# This (optional) file defines metadata for the node parameters.benefits in the parameter tree.

description: Social benefits
documentation: This (optional) file defines metadata for the node parameters.benefits in the parameter tree.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same than below, a realistic example would be much better.
Maybe a generic definition of social benefits?

Suggestion (extracted from this Wikipedia page)

Government support for the citizens and residents of society. They may be provided to people of any income level, as with social security, but usually it is intended to ensure that everyone can meet their basic human needs such as food and shelter.

@sandcha sandcha requested a review from fpagnoux September 27, 2018 13:02
CHANGELOG.md Outdated
* Minor change
* Details:
- Add `documentation` to parameters: `benefits` node and `benefits/housing_allowance`
- Add `documentation` to `housing_allowance` variable and docstring to its formula
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stay at a functional level, I'd just write:

Add documentation to housing_allowance variable and its formula

as the docstring is also documentatiob

@sandcha sandcha force-pushed the editorialized-model-le124 branch from 86ae286 to f88bf29 Compare October 1, 2018 10:14
@sandcha sandcha force-pushed the editorialized-model-le124 branch from f88bf29 to 9da2ca1 Compare October 1, 2018 16:25
@fpagnoux fpagnoux force-pushed the editorialized-model-le124 branch from 879e049 to ec275df Compare October 2, 2018 08:41
@fpagnoux fpagnoux merged commit 7ef65b3 into master Oct 2, 2018
@fpagnoux fpagnoux deleted the editorialized-model-le124 branch October 2, 2018 09:01
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.

3 participants