-
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
Support str
and/or date
parameters
#1092
Comments
I do not recall an obvious reason. For parameters changing on another dimension than the legislation time (period), and specifically parameters depending on a date (like the date of birth for pension modelling), I started something ugly in #978 but worth looking at. |
Thanks @benjello - that PR is very cool! |
@nikhilwoodruff : IIRC there was also a problem with starting a parameter name with a number since it will be somehow an attribute (to allow the use of the dotted noation of the parameters) and an attribute name cannot start wit a number? |
Interesting @benjello - though hasn't OpenFisca already made an made an exception for this via the ParameterScale? |
@nikhilwoodruff : could you provide an example. |
Sure, I meant how e.g. in OpenFisca-UK the second rate of income tax has the name |
It could have been more convenient to have : |
I do not find a good documented reason for not supporting String or Date types in parameters. However, I am a bit concerned with adding such a feature lightly. Extending parameters to a whole new class seems likely to yield many side effects: all handlers (Web API, loaders, tests…) have to be fully tested for support of that new type. @nikhilwoodruff could you please document here clearly two use cases for such types? 🙂 From what I understand, the main one would be to store dates, am I correct? If that is the case, could you please provide us with the legislation you want to model and how you would like to do it? Is there any other? I understand this might be frustrating, but there has been a lot of work in evolving the parameter declaration developer experience, and I believe it is important we keep it light and consistent 🙂 |
Thanks for the clarification @MattiSG - OK, so admittedly the date usage I originally had in mind is not a legislative parameter (part of a current-date feature of PolicyEngine) and so I don't see that being helpful in justifying this. But what about these, which would be an improvement to represent closer to how they are specified in legislation for the UK:
|
Coming back to this, it might be relevant to consider how the current parameter allowed types include lists (including lists of strings). I think there are very clear legislative use-cases for this: for example, income definition lists for means tests or taxes. On the UK and US models, given the legislative examples above, I think we'll use the list parameter as an intermediate workaround for string parameter types (using a list of length 1, containing a single string). But this strikes me as less ideal: OpenFisca-Core does support string parameters (and therefore already has the side effects you mention, if I understand this correctly), but just without explicitly documented support. @MattiSG does this make sense? Happy to expand on the examples above too if needed. |
Thanks @nikhilwoodruff! StringsI agree that lists of strings are currently used and useful. Example use cases:
I am thus not sure to understand the difference between “supporting strings” and these examples. Could you please give a code example of how you would implement the “UK country for each county” in a way that is not currently supported by OF-Core? 🙂 You can use the section below as an example of how to present it. DatesAs mentioned earlier, I don't see any good reason not to support dates. Am I correct that, in the Child Tax Credit act you linked to, the implementation you'd like to have would be something like: child_tax_credit:
born_before:
metadata:
unit: date
values:
2016-03-16:
value: 2017-04-06 How would you see this exposed in Python and JSON? Should the Python API expose a Numpy Datetime object (i.e. hydrate the parameter to an |
I am very surprised that dates are never used in the OF-FR parameters corpus. I did look for |
@MattiSG : I do not recall such examples but we can actually use dates. For example, there are some family allowance that are dismantled and only the kids born before a specific date can apply. |
Thanks @benjello! Just to be 100% sure: when you say “we can actually use dates”, you mean that the French law can do it, right? Not that OpenFisca-Core can currently do it, or that OpenFisca-France already uses it? 😅 |
Yes @MattiSG we could benefit from such a feature but we do not use it right now in our implementation IIRC. |
Is there a technical reason why OpenFisca-Core doesn't allow string or date types in parameters? When I try to add one, it fails the validation check in the parameter initialisation. Some legislative parameters are date-based - e.g. pension changes often affect only those born after a specific date. Happy to file a PR adding this if it's not objectionable.
The text was updated successfully, but these errors were encountered: