-
Couldn't load subscription status.
- Fork 30
Refactor #416
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
Refactor #416
Conversation
… breaking tests for some reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ready for review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, thanks @MaxGhenis: this is a massive improvement to code readability (for both humans and machines).
| return household("property_purchased", period) * household( | ||
| "main_residence_value", period | ||
| ) | ||
| property_purchased = household("property_purchased", period) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have thought about this - while this is equivalent to before, I think assigning to variables like this to avoid the auto-formatted brackets mess makes the formulas more readable - will do this in future, thanks.
This should not change any logic, does a few things:
unitto variablesreturnstatementsfrom openfisca_uk.model_api import *meets_marriage_allowance_income_conditions(I had to debug this one when rewriting)