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 metadata field for stocks/flows #1104

Open
nikhilwoodruff opened this issue Jan 12, 2022 · 8 comments
Open

Add metadata field for stocks/flows #1104

nikhilwoodruff opened this issue Jan 12, 2022 · 8 comments

Comments

@nikhilwoodruff
Copy link
Contributor

We're implementing this in the UK and US countries, but I thought it may be general enough to contribute to Core. Essentially, we'd find use in a metadata field for variables to specify whether they represent a stock (age, wealth) or a flow (income, expenditure), mostly to label them better on the front end. Is this a field we could add to variable.py?

@benjello
Copy link
Member

No objection.

cc @MattiSG @sandcha

@nikhilwoodruff
Copy link
Contributor Author

Probably not general enough to include in Core; we should use existing fields and/or monkey-patch in openfisca-tools.

@sandcha
Copy link
Collaborator

sandcha commented Feb 9, 2022

Probably not general enough to include in Core; we should use existing fields and/or monkey-patch in openfisca-tools.

This was decided because:

  • an attribute is needed in the model when it's used in the calculation or absolutely necessary to understand what part of the law is modeled

    context: for now, openfisca-core and models try to combine the minimum common elements of the different use cases while allowing some free space to try new things; a front end attribute might not be shared/seems specific to a use case.

  • stocks/flows attribute wouldn't be used in the calculation ; it is unclear whether it is absolutely necessary to understand the modeled element (may be not)
  • the free space for trying new things in the variables is the documentation attribute (multiline string); it could be used to test the stocks/flows but calling documentation for this didn't seem natural and you have another way to do it in openfisca-tools
  • variables might be missing some metadata attribute like the parameter metadata where today, any key could be added but this needs a specific discussion with structured examples and some vote from the community (RFC issue) cc @MattiSG

@MattiSG
Copy link
Member

MattiSG commented May 5, 2022

This would be a use case for #1071.

@nikhilwoodruff
Copy link
Contributor Author

nikhilwoodruff commented May 7, 2022

I agree, but should individual metadata attributes be considered as part of it? I'm thinking of unit, which is very much a metadata attribute for variables, but it's its own attribute rather than being in a metadata dictionary.

@MattiSG
Copy link
Member

MattiSG commented May 10, 2022

should individual metadata attributes be considered as part of it

Very good question!

To me, if the use case is:

mostly to label them better on the front end

…then it is pure metadata and should be labeled as such, as opposed to attributes that should be relevant (if not necessary) for computation. This helps ensure the test coverage is appropriate for the criticality of each type, maintainers have clear guidelines for accepting PRs as being complete or not, and contributors focus their efforts.

I'm thinking of unit, which is very much a metadata attribute for variables, but it's its own attribute

Excellent remark! I assume though that you meant “metadata attribute for parameters” (not for variables).
I agree with you that this is inconsistent, hence the RFC in openfisca/openfisca-france#1672 (comment) for switching this parameter metadata to the metadata node instead of the parameter node itself. Considering the votes, it has been accepted for that corpus and should trigger an RFC for Core 🙂

@nikhilwoodruff
Copy link
Contributor Author

Thanks @MattiSG for this helpful clarification! I actually was referring to the Variable unit attribute, am I understanding it correctly?

@MattiSG
Copy link
Member

MattiSG commented May 10, 2022

Hah! Thanks for pointing this out. I don't know in which country packages that one is used. The same reasoning applies there 🙂

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

No branches or pull requests

4 participants