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

Handle values defined on different period structures #934

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

guillett
Copy link
Member

@guillett guillett commented Jan 3, 2020

Based on #933
Fixes #564

The new function solve_input_data is hacky but I would like to start a conversation to have that feature in Core soon.

@Morendil
Copy link
Contributor

Morendil commented Jan 4, 2020

This is not the way to go IMO, it makes Holder more complex, when what we want is for them to become simpler… and even simpler, until eventually they go away. Getting rid of Holders is high priority in improving the architecture of Core.

Ask yourself - why does this need to be in Holder, where it will be in common use among fine-grained YAML-test-style data and tabular data, when only the former needs it ?

Do the Cache API first, migrate to it, get rid of Holders, and then it will be more obvious how to implement this where it should be implemented - in SimulationBuilder.

@guillett
Copy link
Member Author

guillett commented Jan 6, 2020

@Morendil many thanks for your feedback. There are multiple issues to be tacked with such a feature/bug fix

  • have a working version of it
  • put it in the right place.

I worked in the first one we'll have to work on the second one as a community.
@benjello I am eager to get your insight/feedback on that PR too. :)

@benjello
Copy link
Member

benjello commented Jan 8, 2020

@guillett : I do understand the need for a feature handling different period range for the same input_variable and I see how it can be useful.
I am not versed in this part openfisca-core code enough to make any insightful comment at this point, sorry about that.

I would suggest not to use camel case and use more long names for local variables so it can be more readable by a unexperienced coder as myself and I would definitively give it a second try since this question is important.

@bonjourmauko bonjourmauko added the kind:fix Bugs are defects and failure demand. label Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:fix Bugs are defects and failure demand.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle individuals using different period structures for the same variable.
4 participants