Skip to content

Conversation

@MaxGhenis
Copy link
Collaborator

@MaxGhenis MaxGhenis commented Dec 24, 2021

This should not change any logic, does a few things:

  • Add unit to variables
  • Avoid redundant variables just before return statements
  • Other changes to simplify logic
  • Aesthetic coding changes
  • Replacing import statements in variables with from openfisca_uk.model_api import *
  • Add unit test for meets_marriage_allowance_income_conditions (I had to debug this one when rewriting)

Copy link
Collaborator Author

@MaxGhenis MaxGhenis left a 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

@MaxGhenis MaxGhenis linked an issue Dec 25, 2021 that may be closed by this pull request
Copy link
Collaborator

@nikhilwoodruff nikhilwoodruff left a 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)
Copy link
Collaborator

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.

@nikhilwoodruff nikhilwoodruff merged commit faff007 into master Dec 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use ~ instead of not_ or np.logical_not

2 participants