search

gadget-framework / gadget3

TMB-based gadget implemtation
GNU General Public License v2.0
8 stars 6 forks source link

ensure project_years is always a parameter #64

Closed willbutler42 closed 2 years ago

willbutler42 commented 2 years ago

If the project_years argument is numeric, it is substituted into a g3_param formula. Ensures it is always present in the parameter template.

lentinj commented 2 years ago

What's the use-case for this? I can't picture when I might say "actually, I want to default to 5 years projection" when building the model, rather than modifying the parameter template. I'm not sure why you'd want a non-zero default full stop to be honest.

Regardless, if you did, you can still make your custom project_years a parameter.

willbutler42 commented 2 years ago

Yes, I agree with that. I was thinking more along the lines of - if someone supplies an integer to g3a_time for project_years then it will still be in the parameter template to access later, rather than why someone might do that. Maybe a better solution is to remove the project_years parameter from the function call and have it defined as a parameter within so it can only be modified via the parameter template?

lentinj commented 2 years ago

Maybe a better solution is to remove the project_years parameter from the function call and have it defined as a parameter within so it can only be modified via the parameter template?

Yeah, that's definitely a nicer option. But we've gone for flexibility vs. hiding footguns in pretty much all the other built-in methods, it seems odd to start now.

An error message in g3_retro explaining what went wrong and how to fix it if it can't find project_years is probably the best solution though, IMO. It should be checking it exists anyway, and if you don't need g3_retro then you still have the option of messing with project_years.

EDIT: https://github.com/gadget-framework/gadgetutils/commit/a484543b9387d986104ddafbee08033b21d0f12a

willbutler42 commented 2 years ago

Sounds good and thanks for the commit to gadgetutils. Will close the request now