openfisca-core icon indicating copy to clipboard operation
openfisca-core copied to clipboard

Add metadata field for stocks/flows

Open nikhilwoodruff opened this issue 4 years ago • 8 comments

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?

nikhilwoodruff avatar Jan 12 '22 13:01 nikhilwoodruff

No objection.

cc @MattiSG @sandcha

benjello avatar Jan 17 '22 13:01 benjello

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

nikhilwoodruff avatar Feb 09 '22 14:02 nikhilwoodruff

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

sandcha avatar Feb 09 '22 15:02 sandcha

This would be a use case for #1071.

MattiSG avatar May 05 '22 15:05 MattiSG

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.

nikhilwoodruff avatar May 07 '22 17:05 nikhilwoodruff

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 🙂

MattiSG avatar May 10 '22 12:05 MattiSG

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

nikhilwoodruff avatar May 10 '22 14:05 nikhilwoodruff

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

MattiSG avatar May 10 '22 14:05 MattiSG