openfisca / openfisca-core

OpenFisca core engine. See other repositories for countries-specific code & data.
https://openfisca.org
GNU Affero General Public License v3.0
168 stars 75 forks source link

Add metadata field for stocks/flows #1104

Open nikhilwoodruff opened 2 years ago

nikhilwoodruff commented 2 years ago

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 commented 2 years ago

No objection.

cc @MattiSG @sandcha

nikhilwoodruff commented 2 years ago

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

sandcha commented 2 years ago

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

This was decided because:

MattiSG commented 2 years ago

This would be a use case for #1071.

nikhilwoodruff commented 2 years ago

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 commented 2 years ago

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 https://github.com/openfisca/openfisca-france/issues/1672#issuecomment-940117563 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 commented 2 years ago

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

MattiSG commented 2 years ago

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